Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update loops #446

Open
wants to merge 3 commits into
base: development
Choose a base branch
from

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