-
Notifications
You must be signed in to change notification settings - Fork 207
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
DOCN aquaplanet from file and new A compsets with DOCN modes #1582
Conversation
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 can't say I have given this a detailed review, but at a glance it looks fine to me; but see the one line comment.
<lname>2000_DATM%NYF_SLND_DICE%SSMI_DOCN%SOM_DROF%NYF_SGLC_SWAV_TEST</lname> | ||
</compset> | ||
|
||
<compset grid="a%1.9x2.5|a%0.9x1.25"> |
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.
Does this grid attribute actually accomplish anything? I tried creating test SMS.f10_f10.ADSOMAQP.roo2_gnu
and it got through initial case creation just fine, making me think that the grid attribute wasn't doing anything. (I had been expecting the grid attribute to stop me from creating a case with this compset unless I'm using one of the above grids, but that doesn't seem to be the case.)
My understanding was that it should be stopping you from creating a case -
so if it does not - I think this is a bug. Thanks for checking this.
…On Sun, May 21, 2017 at 6:36 AM, Bill Sacks ***@***.***> wrote:
***@***.**** requested changes on this pull request.
I can't say I have given this a detailed review, but at a glance it looks
fine to me; but see the one line comment.
------------------------------
In src/drivers/mct/cime_config/config_compsets.xml
<#1582 (comment)>:
> @@ -46,6 +46,26 @@
</compset>
<compset>
+ <alias>ADSOM</alias>
+ <lname>2000_DATM%NYF_SLND_DICE%SSMI_DOCN%SOM_DROF%NYF_SGLC_SWAV_TEST</lname>
+ </compset>
+
+ <compset grid="a%1.9x2.5|a%0.9x1.25">
Does this grid attribute actually accomplish anything? I tried creating
test SMS.f10_f10.ADSOMAQP.roo2_gnu and it got through initial case
creation just fine, making me think that the grid attribute wasn't doing
anything. (I had been expecting the grid attribute to stop me from creating
a case with this compset unless I'm using one of the above grids, but that
doesn't seem to be the case.)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1582 (review)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AHlxE6WvacjXrCERFozQvYTSh6_We7gYks5r8C_lgaJpZM4Nhfhw>
.
|
I checked it because I didn't remember ever seeing |
Thanks. Can you please create an issue with this and mark it critical.
…On Sun, May 21, 2017 at 11:18 AM, Bill Sacks ***@***.***> wrote:
My understanding was that it should be stopping you from creating a case -
so if it does not - I think this is a bug. Thanks for checking this.
I checked it because I didn't remember ever seeing <compset grid=...>
before. But a quick grep through the code shows that it is used in other
places. So I'd say this is not a problem with this PR, but rather (maybe) a
more general bug. So I approve these changes.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1582 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AHlxE8P21kPeV7BLQlIsDgEV3YCmD0vKks5r8HHegaJpZM4Nhfhw>
.
|
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.
Clear up what is happening to copyall. Mention in PR description.
@@ -15,25 +15,25 @@ | |||
|
|||
<entry id="DOCN_MODE"> | |||
<type>char</type> | |||
<valid_values>prescribed,sst_aquap1,sst_aquap2,sst_aquap3,sst_aquap4,sst_aquap5,sst_aquap6,sst_aquap7,sst_aquap8,sst_aquap9,sst_aquap10,som,som_aquap,copyall,interannual,null</valid_values> |
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.
Did you mean to get rid of "copyall"
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.
Yes. copyall
was not being used so having it there was simply confusing.
@@ -254,7 +256,7 @@ | |||
<type>char</type> | |||
<category>streams</category> | |||
<group>shr_strdata_nml</group> | |||
<valid_values>SSTDATA,SST_AQUAP1,SST_AQUAP2,SST_AQUAP3,SST_AQUAP4,SST_AQUAP5,SST_AQUAP6,SST_AQUAP7,SST_AQUAP8,SST_AQUAP9,SST_AQUAP10,SOM,SOM_AQUAP,IAF,NULL,COPYALL</valid_values> | |||
<valid_values>SSTDATA,SST_AQUAP1,SST_AQUAP2,SST_AQUAP3,SST_AQUAP4,SST_AQUAP5,SST_AQUAP6,SST_AQUAP7,SST_AQUAP8,SST_AQUAP9,SST_AQUAP10,SST_AQUAPFILE,SOM,SOM_AQUAP,IAF,NULL,COPYALL</valid_values> |
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.
COPYALL wasn't removed here but is removed in other parts of this PR
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.
The datamode in namelist_definition.xml and the DOCN_MODE are not the same thing. The DOCN_MODE maps to a datamode that is used directly by the docn model. For now DOCN_MODE copyall is not used - so I removed it. But the DOCN datamodel value of COPYALL could still be used at some point - so it should stay in the valid values.
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.
Ok. Just update the PR to mention that COPYALL is being removed from DOCN_MODE.
@@ -0,0 +1,2 @@ | |||
./xmlchange HIST_OPTION=ndays | |||
./xmlchange HIST_N=1 |
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.
Not sure what github is complaining about here. Missing end-of-file?
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 took that symbol to mean "no newline at end of file"
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 just added a new line.
@billsacks - I have just addressed the comments made by rljacob - so I think this is ready to be merged. |
Update LANL IC machines to reflect recent change from moab to slurm This PR updates machine and batch files to reflect the recent LANL machines job scheduler change from moab to slurm. It includes changes for machine grizzly from branch vanroekel/LANLIC_grizzlySlurmTransition as well as changes for machine wolf. This PR also includes adding a default pe-count for wolf to reflect new minimum core count under slurm. Tested with: -compset CMPASO-NYF -res T62_oEC60to30v3 -mach wolf [BFB]
Update LANL IC machines to reflect recent change from moab to slurm This PR updates machine and batch files to reflect the recent LANL machines job scheduler change from moab to slurm. It includes changes for machine grizzly from branch vanroekel/LANLIC_grizzlySlurmTransition as well as changes for machine wolf. This PR also includes adding a default pe-count for wolf to reflect new minimum core count under slurm. Tested with: -compset CMPASO-NYF -res T62_oEC60to30v3 -mach wolf [BFB]
Update LANL IC machines to reflect recent change from moab to slurm This PR updates machine and batch files to reflect the recent LANL machines job scheduler change from moab to slurm. It includes changes for machine grizzly from branch vanroekel/LANLIC_grizzlySlurmTransition as well as changes for machine wolf. This PR also includes adding a default pe-count for wolf to reflect new minimum core count under slurm. Tested with: -compset CMPASO-NYF -res T62_oEC60to30v3 -mach wolf [BFB]
Addition of file driven aquaplanet to DOCN
This PR actually does several things:
adds aquaplanet capability to DOCN where the input is from a file rather than analytic.
adds a new directory
$CIMEROOT/src/drivers/mct/cime_config/testdefs/testmods_dirs/drv/default
that creates cpl history files on a daily basis and thereby permits answer changes to be compared more easily.
adds new A compsets to the driver config_compsets.xml that have different DOCN functionality
ADSOM - DOCN SOM
ADSOMAQP - DOCN aquaplanet SOM
ADAQP3 - DOCN analytic aquaplanet (mode 3)
ADAQFILE - DOCN aquaplanet from file
removes COPYALL as a possible DOCN_MODE since it is not used
Test suite: scripts_regression tests and
created a modified sandbox off of
8657a18
with the new compsets and default/ testmods directory to show that the following tests were bfb with8657a18
Test baseline:
8657a18
Test namelist changes:
Test status: bit for bit
Fixes #1581
Fixes #1527
User interface changes?: None
Code review: