-
Notifications
You must be signed in to change notification settings - Fork 61
Bring in recent changes from NorESM #364
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
Conversation
update to cdeps1.0.28 Description of changes: Update the NorESMhub CDEPS code to ESCOMP cdeps1.0.28. Specific notes: There are no answer differences between this merge and cdeps1.0.28 This merge is needed in order to begin the creation of a data GLC component in cdeps. This PR adds U10m and V10m fields to the fields in cdeps1.0.28 Contributors other than yourself, if any: None CDEPS Issues Fixed: None Are there dependencies on other component PRs (if so list): Yes - the following PR needs to be merged before CDEPS can build successfully. NorESMhub/NorESM_share#3 Are changes expected to change answers: No The update to CDEPS causes the default docn sst files to be different and therefore using this update will cause differences in answers. Any User Interface Changes (namelist or namelist defaults changes): None Testing performed (e.g. aux_cdeps, CESM prealpha, etc): Using the following external - new baselines were created in Using the hash f71e2c7 in the branch feature/oslo_aero_camdev from https://github.com/mvertens/NorESM.git and running the aux_cam_noresm test suite - new baselines were created noresm_v11_cam6_3_123_cdeps/. All tests passed functionality - but comparisons to noresm_v11_cam6_3_123_cdeps failed.
Update to cdeps1.0.32 from ESCOMP - Add a new DGLC component (see #268) Contributors other than yourself, if any: None CDEPS Issues Fixed: None Are there dependencies on other component PRs (if so list): Yes See documentation in #268 Are changes expected to change answers: No, verified as BFB Any User Interface Changes: only for DGLC Testing performed Ran prealpha_noresm test suite along with aux_cdeps_noresm successfully Hashes used for testing: Externals in what will be noresm2_5_alpha02
update to cdeps1.0.33
update to ESCOMP cdeps1.0.49
Rpointers update cdeps
Updates from ESCOMP
Updates from ESCOMP
Copy shr_lnd2rof_tracers_mod from noresm branch ### Description of changes Copying from noresm branch at NorESMhub/CMEPS@6539501e This is needed for ESCOMP/CDEPS#364 We should soon try to bring in all of the changes from the noresm branch, but that will take more work. ### Specific notes Contributors other than yourself, if any: Mariana Vertenstein CMEPS Issues Fixed (include github issue #): Are changes expected to change answers? (specify if bfb, different at roundoff, more substantial) : no Any User Interface Changes (namelist or namelist defaults changes)? none ### Testing performed Please describe the tests along with the target model and machine(s) If possible, please also added hashes that were used in the testing Ran `SMS_Ld3_D_P8x1.f10_f10_mg37.X.green_gnu` with the changes from ESCOMP/CDEPS#364
|
I have tested the changes here along with the CMEPS changes in ESCOMP/CMEPS#618 in @fischer-ncar - despite this GitHub actions failure, I think this is ready for prealpha testing whenever you get a chance. You'll need the latest version of cmeps (from the merged ESCOMP/CMEPS#618) along with this branch. We expect this to be bit-for-bit, so especially want to know if your testing turns up any answer changes. Thank you! |
|
I just started the prealpha tests. |
|
In the past CDEPS has not had any dependancy on CMEPS. I think the solution here is to copy the file shr_lnd2rof_tracers_mod.F90 into the CDEPS/share directory. |
|
@jedwards4b - I am not sure this is the correct thing to do - since clm also needs this shr_lnd2rof_tracers and will fit in with what @billsacks is going to be doing for water isotopes. Can we please chat about this before you actually move the code? |
|
I didn't move it. I copied it and this is what is done with other files from that directory as well. |
|
Thanks, @jedwards4b . Long-term I hope we can come up with a different solution that avoids these copies but for now this feels like the right path forward - thanks a lot! |
|
Here's the prealpha test results. There were several CREATE_NEWCASEs that failed for the same reason, error message below. There's a couple of tests that had a difference in fill patterns. And the last test had answer changes. Which I'm guessing is caused by a change in the namelist. I'm going to run the nag prealpha tests to see if anything pops out there. FAIL ERS.TL319_t232.G_JRA.derecho_gnu.cice-default CREATE_NEWCASE
FAIL SMS_D_Ly1.f09_g17_ais8.T1850Ga.derecho_gnu BASELINE cesm3_0_alpha08a: DIFF
FAIL SMS_D.TL319_t232.G_JRA_RYF.derecho_intel BASELINE cesm3_0_alpha08a: DIFF
You can access the tests in /glade/derecho/scratch/fischer/t/cesm3_0_alpha08a.NorESM_i and /glade/derecho/scratch/fischer/t/cesm3_0_alpha08a.NorESM_g. |
|
NAG tests passed. |
|
Thanks a lot for this test summary, @fischer-ncar ! I talked with @mvertens about this. @mvertens is going to fix the CREATE_NEWCASE failures in various G/C compsets, which appear to be due to a change in drof where something that should have been an addition was done as a replacement of a previous mode. @mvertens will also look into the baseline failure in G_JRA_RYF. I took a quick look at this, and it looks to me like this might actually be a desired bug fix: it looks like this compset may have been picking up the wrong mode before and may now be fixed to pick up the correct mode. But I'd like to hear from @mvertens whether this seems right, because I haven't looked carefully. And if this does seem like the right explanation, then I'd like to get confirmation from the ocean group that this change is desired. I looked into the T compset diffs. As @fischer-ncar said, the differences are just in fill patterns. These diffs are just in the lndImp fields - not in any export fields to glc, or in any CISM history fields. It looks like this is coming from this new code in dlnd: if (lfrac(n) == 0._r8) fldptr2(:,n) = 1.e30_r8Given that this isn't causing differences in the CISM fields, this seems like an acceptable change - and probably desirable, since now ocean points show up as missing values rather than 0s in the dlnd -> mediator fields. @mvertens can you confirm that this is an intentional change? If so, I think we should sign off on it and just keep @Katetc in the loop (Kate, let me know if you want me to summarize the context here so you don't need to read back through all of this). |
|
Thank you for the drof fix, @mvertens - that looks good to me now! Once you get a chance to look at the baseline failure in G_JRA_RYF, and to sign off on the T compset changes, my thought is to just have @fischer-ncar rerun the C and G tests from the prealpha suite. @fischer-ncar and @mvertens please share if you have thoughts on that. |
|
I would prefer to rerun the prealpha tests on derecho. It's not that much more work and it'll catch anything that's unexpected. I don't feel the need to rerun the nag tests on izumi. |
|
@fischer-ncar @billsacks - for
So in this case the correct |
|
@mvertens - thank you for your comment. This agrees with my sense. I'll reach out to some ocean folks here to make sure they're aware of this upcoming change. But I think it's safe to move ahead here... if for some reason there is an objection to this change, we can back it out, but I want to keep things moving. @fischer-ncar - sounds good on just rerunning the full prealpha suite. I think you can go ahead and do that now, and if things look good (with some expected DIFFs that you noted before) then we can merge this. |
|
The prealpha tests look good. |
|
Thanks a lot, @fischer-ncar ! I'll bring this in now. |
Description of changes
I'm opening this PR on behalf of @mvertens , so that we can split #363 into pieces to facilitate review.
Changes here include:
datm:JRA-RYF9091, rather than using the genericCORE_IAF_JRAmodedocn:dlnd:Specific notes
Contributors other than yourself, if any: @mvertens @gold2718 @mvdebolskiy @TomasTorsvik @matsbn
CDEPS Issues Fixed (include github issue #):
Are there dependencies on other component PRs (if so list):
Are changes expected to change answers (bfb, different to roundoff, more substantial): YES, including some more substantial changes:
JRA-RYF9091, rather than using the genericCORE_IAF_JRAmodeAny User Interface Changes (namelist or namelist defaults changes):
Testing performed (e.g. aux_cdeps, CESM prealpha, etc):
Hashes used for testing: