Skip to content

Conversation

dafyddstephenson
Copy link
Collaborator

@dafyddstephenson dafyddstephenson commented Oct 30, 2023

Hopefully these changes address issue 176, look forward to discussing

@mnlevy1981
Copy link
Collaborator

Thanks for the PR! Several of the automated tests are failing -- one is just a formatting check:

+ ./code_consistency.py
Check Fortran files for coding standard violations:
* Check for hard tabs: 0 error(s) found
* Check for trailing white space: 4 error(s) found
  ../src/marbl_interior_tendency_mod.F90:96:   ! Changed calls with "km" as an argument to call with "kmt".█
  ../src/marbl_interior_tendency_mod.F90:410:          delta_z1, tracer_local(:, :), unit_system, dissolved_organic_matter) !!█
  ../src/marbl_interior_tendency_mod.F90:419:     PON_remin(kmt+1:km) = c0█████
  ../src/marbl_interior_tendency_mod.F90:421:     do k = 1, kmt !! tested█
* Check length of lines: 0 error(s) found
* Check for case sensitive statements: 0 error(s) found
* Check for r8: 0 error(s) found
Fortran errors found: 4

Check python files for coding standard violations:
* Check for hard tabs: 0 error(s) found
* Check for trailing white space: 0 error(s) found
Python errors found: 0

Total error count: 4

and removing the trailing white space on those four lines will bring it back to passing. You're also failing the call_compute_subroutines regression test, with the following differences being flagged:

 Variable: OtherRemin
... Max relative error (252787706478.10184) exceeds 2e-11

Variable: SedDenitrif
... Max relative error (25629617029027.01) exceeds 2e-11

Variable: ponToSed
... Max relative error (0.0030960605251377496) exceeds 2e-11

and

 Variable: CISO_photo13C_TOT_zint
... Baseline is 0 at some indices where abs value of new data is larger than 1e-16
... Max relative error (829635868.3181239) exceeds 2e-11

Variable: CISO_photo14C_TOT_zint
... Baseline is 0 at some indices where abs value of new data is larger than 1e-16
... Max relative error (8651397805.957771) exceeds 2e-11

Variable: OtherRemin
... Max relative error (2467333713865.9106) exceeds 2e-11

Variable: SedDenitrif
... Max relative error (14858740648012.867) exceeds 2e-11

Variable: calcToSed_13C
... Max relative error (6638000152.180505) exceeds 2e-11

Variable: calcToSed_14C
... Max relative error (7227000710.049543) exceeds 2e-11

Variable: pocToSed_13C
... Max relative error (273780717.5581145) exceeds 2e-11

Variable: pocToSed_14C
... Max relative error (802876004.4584098) exceeds 2e-11

Variable: ponToSed
... Max relative error (17978358.00659296) exceeds 2e-11

Did you have any conflicts to resolve when you merged in the latest development branch (adding the abiotic dic tracers)? This looks like it maybe reverted something we had talked about a few weeks ago.

Once the Github Actions checks are passing, we can talk about scheduling a more thorough code review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants