-
Notifications
You must be signed in to change notification settings - Fork 60
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
(+*) Fix bugs in tracer index in tracer reservoirs #480
Conversation
fix index in the function update_segment_tracer_reservoirs
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #480 +/- ##
============================================
- Coverage 37.79% 37.78% -0.02%
============================================
Files 270 270
Lines 78404 78431 +27
Branches 14521 14525 +4
============================================
+ Hits 29636 29638 +2
- Misses 43364 43389 +25
Partials 5404 5404
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks you @WenhaoChen89 for these modifications which allow for the tracer registry and the obc segment tracer registry to function as intended with arbitrary tracer ordering. I agree with the interface modifications as proposed. Given the dependencies involved with other tracer code such as generic_tracers, additional approvals are requested from @nikizadehgfdl
8a66adc
to
9de6ce7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have examined these changes and I am convinced that they are correct.
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/20903 ✔️ Squash-merging due to development commits. |
Sorry, I think that I accidentally closed this rather than merge it. Handling it now. |
The tracers in the tracer registry (tracer_registry_type), the segment tracer registry (OBC_segment_tracer_type), and the OBC input fields (OBC_segment_data_type) are different. Therefore, they are using different indices and orders. However, in the function update_segment_tracer_reservoirs and tracer advection, all tracers including OBC_segment_tracer_type and OBC_segment_data_typeare looped through the indices and orders of the tracer_registry_type. This pull request is to fix this bug.