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

Cesm2 1 x rel #38

Merged
merged 3 commits into from
Jun 29, 2020
Merged

Cesm2 1 x rel #38

merged 3 commits into from
Jun 29, 2020

Conversation

mnlevy1981
Copy link
Collaborator

@mnlevy1981 mnlevy1981 commented Jun 26, 2020

Description of changes:

Bring Keith's changes from #36 to master for CESM 2.2

Testing:

Test case/suite: aux_pop on cheyenne_intel
Test status: bfb with cesm_pop_2_1_20200603; the 2 failures were both from a new test

	1	NLCOMP (FAIL)
      1 ERS_Ld5_D.T62_g37.C.cheyenne_intel.pop-abio_dic_dic14_ltavg_NK
	1	BASELINE (FAIL)
      1 ERS_Ld5_D.T62_g37.C.cheyenne_intel.pop-abio_dic_dic14_ltavg_NK

User interface (namelist or namelist defaults) changes?

computation and inclusion of vars in tavg file controlled with nml var abio_dic_dic14_ltavg_NK
(nml var not written to pop_in, just passed to tavg_contents generating script)

For testing, I resolved the conflict in the merge with git rm BranchChangeLog; I suspect the file will still be displayed in the PR interface here on github, but it will not be added master

klindsay28 and others added 3 commits June 21, 2020 05:45
Testing: passes aux_pop on cheyenne/intel, compared to pop2_cesm2_1_rel_n10
    expected NLCOMP & BASELINE failures for new test
    some MEMCOMP failures

computation and inclusion of vars in tavg file controlled with nml var abio_dic_dic14_ltavg_NK

nml var enabled in new test ERS_Ld5_D.T62_g37.C.cheyenne_intel.pop-abio_dic_dic14_ltavg_NK

mv io_read_fallback_register_field call for 'ABIO_PH_SURF' outside of all_fields_exist_in_restfile conditional
@mnlevy1981 mnlevy1981 requested a review from klindsay28 June 26, 2020 20:32
@mnlevy1981 mnlevy1981 marked this pull request as ready for review June 26, 2020 21:43
@mnlevy1981
Copy link
Collaborator Author

@klindsay28

I know I had asked about reviewing this and possibly meeting over zoom to make sure nothing unexpected snuck in... but I think I convinced myself that we don't really need to do that:

  1. I saved the diffs between the last two pop2_cesm2_1 tags to foo

    $ git diff pop2_cesm2_1_rel_n10 pop2_cesm2_1_rel_n11 > foo
    
  2. I saved the diffs from this merge (after removing BranchChangeLog) to bar

    $ git diff --cached > bar
    
  3. I edited foo to remove the BranchChangeLog diffs and ensure the files are listed in the same order as in bar

  4. The diff makes me think the merge was fine

    $ diff foo bar
    2c2
    < index b239048..8f00dce 100755
    ---
    > index 0b74953..f485bb8 100755
    5c5
    < @@ -2751,6 +2751,9 @@ sub module_tavg_script_args() {
    ---
    > @@ -2717,6 +2717,9 @@ sub module_tavg_script_args() {
    16c16
    < index 7da15dd..838f78a 100644
    ---
    > index d0af8b7..3e68264 100644
    33c33
    < index 3a05036..56ec558 100644
    ---
    > index 055614f..a9a2032 100644
    58c58
    < index 78fb54c..283fcde 100644
    ---
    > index 2693a6e..3f7e896 100644
    

Between this and the aux_pop test results I'm happy to merge without meeting to talk about it, but if you want to chat the offer still stands. Could you please let me know either way?

Copy link
Collaborator

@klindsay28 klindsay28 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thanks for the careful diffs.
I don't think we need to have a call on this one.

@mnlevy1981 mnlevy1981 merged commit 9cad4c4 into master Jun 29, 2020
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