-
Notifications
You must be signed in to change notification settings - Fork 53
Rename cice to seaice #326
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
@akturner and @milenaveneziani, it would be good to test on a new E3SM run that includes the name change. I would be happy to run a QU240 G case and test with the results, assuming the current master branch of e3sm would produce the desired result. @akturner, is that the case? One other thing that needs to be tested is if existing runs (before this change) still work as long as we explicitly give the old names for the namelist and streams files (as I have done in this PR). I will do some testing but would appreciate some from you, @milenaveneziani, if you're able. One other bit of cleanup that we could include in this PR is that some of the runs we refer to in the example config files are no longer available (archive or deleted by @golaz). We will need to choose some newer runs as a substitute, probably from the v1 DECK runs currently being performed. I'll work on this when I can. |
@akturner: will the change to MPAS-Seaice go in before the release? |
@milenaveneziani: I believe so, Jon can confirm @jonbob: Jon has confirmed, this is going into the release. |
@xylar: Not in master yet, this is the PR to do so: E3SM-Project/E3SM#2229 |
Thanks for the clarification. @milenaveneziani, I think an a-prime PR should be prepared to handle this ASAP. I believe the current MPAS-Analysis would work unchanged with this change so A-Prime should be able to be updated to support the change without this PR. I also think it would be easy to create a version of A-Prime that is backwards compatible so I don't see any downside to addressing this before E3SM-Project/E3SM#2229 gets merged. |
well, but we need at least a E3SM test run for a-prime. So things have to move in parallel with E3SM-Project/E3SM#2229, I think. |
This really isn't the right place for this discussion. Let's move to the issue I created in A-Prime. |
@milenaveneziani, same question to you I guess. |
Hi, So I'm a little confused what Rob Jacob has actually done. I think he's allowed the changes to the namelist and streams files but not other ones. If this supports the old and new I think we should proceed with this, because we plan to change the output with v2.1. Is that ok? |
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 don't have a good enough understanding of the code to approve the actual code changes. If this supports a mixture of the old and new names (Rob only allowed some of the changes) then I approve.
streams and namelist filenames have changed for sure. What I know hasn't changed are the compset names, which don't matter for this PR, of course. |
* `mpascice.rst.0002-01-01_00000.nc` (or any other mpas-cice restart | ||
* mpas-seaice files: | ||
* `mpasseaice.hist.am.timeSeriesStatsMonthly.*.nc` | ||
* `mpasseaice.rst.0002-01-01_00000.nc` (or any other mpas-seaice restart |
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.
For the time being, I would keep the old filenames in there as well. We can remove them all together when the change to MPAS-seaice is definite.
* `streams.cice` | ||
* `mpas-cice_in` | ||
* `streams.seaice` | ||
* `mpas-seaice_in` |
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.
same goes with these.
Okay, sounds good. I can make clear that some runs will have the old ( |
652b16f
to
c5abde7
Compare
@milenaveneziani, could you make sure the changes I made address your concerns? |
@xylar, for some reason, I don't see the new changes. |
I just amended the original commit. |
* `mpascice.rst.0002-01-01_00000.nc` | ||
* `streams.cice` | ||
* `mpas-cice_in` | ||
|
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.
@milenaveneziani, here are the additions that you requested.
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.
great thanks.
``streams.cice``, (even if they will not be written out -- | ||
``output_interval=="none"``):: | ||
``streams.seaice`` or ``streams.cice`` in older E3SM runs, (even if they will | ||
not be written out -- ``output_interval=="none"``):: |
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.
@milenaveneziani, you didn't explicitly request this but I also made changes here and on line 17 above to reference older E3SM runs.
I will test this on an existing E3SM run today. |
The high-res G case seems like a heavy test. Maybe @darincomeau could help us create a mock-up of the name change on his low-res G case on Theta: |
@xylar sure - what did you have in mind? Did you want me to run this on the theta low-res G case as is, to test that it works with cice names? Or rename cice files to seaice, and test that? Or both? |
@darincomeau: I think you can just create links for streams.seaice and mpassi_in to streams.cice and mpas-cice_in, respectively, and you should be done. |
@darincomeau, yes, running as is on your run would not hurt. You will probably need to add the old names of the MPAS-Seaice namelist and streams files to your config file because the defaults have changed.
Then, you could create the links above as @milenaveneziani says, though the correct names are:
and you would remove (or edit) the two config entries above so it uses the new names. (This is kind of a silly test because I'm sure it will work...) |
@xylar , @milenaveneziani , I've just finished running this on my theta run with both mpas-cice_in, streams.cice and mpas-seaice_in, streams.seaice listed in the config file, and both ran without problem. Let me know if there's anything else you'd like me to test. |
Glad to hear it. I think that should be it. @milenaveneziani, can I merge or did you also want to test? |
Everywhere except where used by ncclimo. Use old input and streams file names in config files for existing E3SM.
c5abde7
to
c4f1d21
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.
no, that's fine. go ahead.
Thanks @darincomeau.
Thanks! |
...everywhere except where used by ncclimo and in namelist and streams files in config files for existing E3SM runs.
Following @akturner's work to rename MPAS-Seaice (from MPAS-CICE), we need to make the same update in MPAS-Analysis. In practice this affects relatively few parts of the code because output file names from E3SM runs are read in from the streams file and we don't happen to use any namelist options that included
cice
in the name.