fix sat hist capability and add test #6641
Conversation
|
|
I am requesting review from @jayeshkrishna and @dqwu since the underlying bug was related to PIO. Please feel free to review or ask someone else. Thanks! |
|
Did you look at the output? |
Yes, and as far as I could tell it makes sense:
plots
black: obs using ship-tracked outputs using large gridbox |
|
@jayeshkrishna and @dqwu please review |
jayeshkrishna
added
BFB
|
@mahf708 : I would also recommend adding some text in the commit (commit message) to explain the change. |
Thanks for the detailed review! Please bear with me as I go through it and address it --- hopefully by tomorrow. My Fortran knowledge is pretty low, but I will do my best to make this a better PR |
Ressurecting a long-forgotten capability to output variable along ship/flight tracks, and adding a test to (weakly try to) ensure it doesn't break again. The code is rewritten such that it uses a different IO logic (more serial) and support is added for different types of output variables. Fixes #6543 [BFB] Note: I don't have full provenance from where the code edits are from. I recall attempting to fix this capability in July and August 2024, and I recall trying different versions of available CAM code to do so. But the final version isn't easily traceable (so it could be a mix of edits from CAM and debug edits to make it work in current EAM). For reference, here's the CAM code: https://github.com/ESCOMP/CAM/blob/cam_cesm2_1_rel/src/control/sat_hist.F90. I don't claim ownership of the code at all, and I am happy to give "authorship" to whomever claims it.
mahf708
force-pushed
the
mahf708/eam/sathis
branch
from
784af44
to
b5c3c7f
Compare
- Updated default filename spec - Handled error from put call - Added note to improve code - Updated test to ERS
|
@mahf708 is this ready? |
|
Yes! I only had a minor mod after @jayeshkrishna last reviewed (should be non-controversial) and I ensured it was BFB with the first commit. Output looks reasonable and ERS test passes (as well as SMS) with the testmod. 🎉 |
Ressurecting a long-forgotten capability to output variable along ship/flight tracks, and adding a test to (weakly try to) ensure it doesn't break again. Fixes #6543 [BFB]
This comment was marked as outdated.
This comment was marked as outdated.
Second merge of this PR. Remove test since its not working on all platforms.
mahf708
force-pushed
the
mahf708/eam/sathis
branch
from
1bca38b
to
9a66d99
Compare
3rd merge of this branch. Reactivate test with smaller sat file.
|
@rljacob I think this is ready to be merged. Let me know if you agree/disagree, thanks. Then I can submit the bless request for the DIFFs on chrys and pm-cpu, as well as gcp |
|
I'll do the blessing. |
Ressurecting a long-forgotten capability to output variable along ship/flight tracks,
and adding a test to (weakly try to) ensure it doesn't break again.
Fixes #6543
[BFB]
Note: I don't have full provenance from where the code edits are from. I recall attempting to fix this capability in July and August 2024, and I recall trying different versions of available CAM code to do so. But the final version isn't easily traceable (so it could be a mix of edits from CAM and debug edits to make it work in current EAM). For reference, here's the CAM code: https://github.com/ESCOMP/CAM/blob/cam_cesm2_1_rel/src/control/sat_hist.F90. I don't claim ownership of the code at all, and I am happy to give "authorship" to whomever claims it.