-
Notifications
You must be signed in to change notification settings - Fork 204
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
Support the creation of multiple user_nl files for a component #3999
Comments
@billsacks What about a separate file for each grid? So you would have user_nl_cism, user_nl_cism.gris and user_nl_cism.ais ? I believe that this implementation could be done with fewer less disruptive changes in cime. |
I agree with @jedwards4b, this feels more like you are running two instances of an ice sheet model so using multiple namelist files feels most CIME-like. |
@jedwards4b @gold2718 - As @whlipscomb pointed out, we will eventually want to support multiple paleo ice sheets: Laurentide, Eurasian, Fennoscandian, and so on. In addition to Greenland (gris) and Antarctica (ais), we'd have to carry around other user_nl_* files for ice sheets that are irrelevant to most people running the model. I believe that this would be awkward for the user and support. What if a user wanted to use data from a previous run - but use less ice sheets. Rather than just editing one user_nl_cism file they would have to separately edit and create only a required subset of files. |
The problem is related to the CIME code for prestaging the user_nl files to the case directory. As alluded to by @whlipscomb and @mvertens, I think it comes down to how much we care about having (potentially many) unnecessary user_nl files staged in the case directory. It would be easy to extend the CIME code that prestages files so that, instead of just staging To be clear, even with the proposed approach of having a single @jedwards4b I'm also wondering if you can explain why you view my proposal as disruptive. It would mean adding one function that could be invoked by components, but wouldn't impact any existing code. That said, I'm fine with keeping this in CISM if you object to its inclusion in CIME. |
@mvertens I don't think that we need to "carry around" any user_nl_ files that are not used for the current case. I think that most of this functionality can be handled in cism's buildnml file, using multiple files will require few if any changes in cime. It is known when buildnml is invoked what ice sheets are active for the case. I think that the distinction you are trying to make between support for multiple files and support for multiple sections in a single file is weak at best. |
@jedwards4b I have a feeling you were writing your reply before mine came through. The tie-in with CIME is the need to initially stage the user_nl files to the case directory, which is something handled by CIME, not by CISM's buildnml. |
I'm sure I've replied to one of these before, but please can you remove my
email from here. I've no idea why my email address got involved with you
guys, but I'm sure you don't want a random external email address involved
in your communications
Cheers,
…On Tue, 15 Jun 2021 at 16:43, Bill Sacks ***@***.***> wrote:
@jedwards4b <https://github.com/jedwards4b> I have a feeling you were
writing your reply before mine came through. The tie-in with CIME is the
need to initially stage the user_nl files to the case directory, which is
something handled by CIME, not by CISM's buildnml.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3999 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAD77NDVDU37DOQK7QT6CELTS5YJRANCNFSM46XJ2IRQ>
.
|
Perhaps this points out a flaw in cime - the component buildnml knows what user_nl_* to expect in a given case, the responsibility to put them there, if not already present, should be added here. I think the addition of new files with the already defined and tested file format is going to be less disruptive than adding a new format modification which may have multiple unintended consequences. |
jedwards - very sorry for your trouble - I don't see how we can remove you from the thread once referenced - you will have to remove yourself using the unsubscribe button on the lower right of the issue page. |
jedwards sorry that was my fault for not adding the "4b". I apologize. |
@jedwards4b - after talking with @mvertens we would really like to move ahead with the originally-proposed solution of having a single user_nl_cism file with multiple sections. We feel this will be most clear / least confusing to users. So the question for you at this point is just whether you object to having a function that parses this file in the CIME lib. I want to reiterate that this function would not be called in the normal / existing flow of code. It would just be available for component buildnmls to call this function if they want. So it has no potential for unintended consequences for other components. That said, if you object to putting it in CIME, I can put it in CISM instead. |
@jedwards4b, I am trying to remove jedwards by editing the comments that had a typo in a reference. |
I think I withdraw my opposition to this change as long as sections are not required. We are moving in a different direction in CAM but I am interested in learning more about your approach.
Where is this file output? Why is a file needed instead of some sort of return object? What is output in the run directory (i.e., one file, multiple files)? |
@billsacks I think that (1) would be easier and require few if any changes in cime, but I don't object if you want to proceed with adding separate_user_nl(...) - I'm curious - what is produced in the run directory, that is what does cism actually read for input? |
I see your rationale, but it seems like this would require a bigger change – introducing a whole new step at which a component-specific script is called, in the create_newcase process.
Right, this would be completely optional and not impact other components in any way.
I would be interested to hear what that direction is – at least if you are solving a similar problem. One (important) rationale I see for putting this new code in CIME is to try to facilitate code sharing between components if other components have substantially similar needs. Also, if two components are solving basically the same problem, it would be nice for users if the user interface mechanism is the same, rather than having arbitrary differences between components. We could adjust our approach in CISM to remain consistent with other components if there is already a very similar thing being done by others.
CISM's buildnml code (as for other components) currently calls CIME's function, cime/scripts/lib/CIME/buildnml.py Lines 98 to 124 in 71ddf1c
(Side note: this argues for putting a new function in this buildnml.py, not in user_mod_support.py.) This function expects a pre-existing file (typically What I'm proposing is keeping effectively that same flow, but now, instead of using the whole contents of
If you are running with N ice sheets, the final result is N+1 files: a |
This would seem to argue for separate files in the case directory as the easiest solution for the user. They would simply copy the file back from the run directory to user_{filename} in the case directory and edit as needed. I think that this is a common practice, especially if the changes are complex. |
I have been thinking about this more, especially inspired by this comment from @jedwards4b:
That idea is growing on me, if (1) @whlipscomb is happy with having multiple Specifically, I think it would be relatively easy to add functionality to CISM's (Q1) @whlipscomb - would you be happy with this solution (i.e., option (1) from ESCOMP/CISM-wrapper#50) if we would only have user_nl files for the ice sheets actually present in this case? @jedwards4b and other cime developers: The implementation I imagine for this is: In cime/scripts/lib/CIME/utils.py Lines 408 to 414 in 71ddf1c
If present, then it calls that function to get a list of (Q2) @jedwards4b and anyone else: does this implementation – and specifically the optional new connection from cime to components' buildnml – seem reasonable? If people like this, then I have two additional questions, for @jedwards4b, @whlipscomb, @mvertens and anyone else: (Q3) Tradeoff between ease of adding a new ice sheet and specificity of explanatory text at the top of each file: I can see two ways to implement this:
The advantage of (1) is that we could have comments specific to this ice sheet. For example, in (Q4) How to name the files: I would propose names like Finally, I'll just point out that this:
doesn't work (I think) for CISM, even with a single ice sheet, because I think that the format of the cism.config file is incompatible with the format of the user_nl file. I also think that this is a bad practice that we should discourage, for a number of reasons (documenting what you've changed in your case, and the potential to accidentally override some setting whose default value depends on another namelist variable's value). |
@billsacks, thanks for continuing to think about the problem carefully. For (Q1), I'm OK with having one user_nl_cism file for changes common to all ice sheets, along with additional files like user_nl_cism_gris for changes unique to a single instance. (Provided we don't have user_nl_* files for ice sheets that aren't part of the case, as you said.) I think this is no harder for users to understand than having multiple sections in a single user_nl_cism file, and might actually be less error-prone. If I understand correctly, a user would have two ways to make a change common to all ice sheets: (1) Change user_nl_cism, or (2) Change both user_nl_cism_gris and user_nl_cism_ais. Is that right? The second way takes more typing but requires editing fewer files. I could imagine that some users might prefer (2). For (Q3), I don't feel strongly one way or the other. For (Q4), my preference is to use the short names: gris, ais, and so on. I'm fine with underscores. Thanks again! |
Thanks @whlipscomb !
That's right. However, we still need the overall We could say that |
@billsacks, I like the idea that changes to user_nl_cism apply to all cism.config files as well as cism_in, and I'm OK with having more than way to change settings for a particular ice sheet. I just wanted to make sure I understood the logic. |
@billsacks This looks good, and I think that the way you are implementing it will allow other components to take advantage of the feature if and when they are ready to do so. Thanks |
Thanks @jedwards4b - and thanks for your great suggestion of having the components' buildnml handle this responsibility. Once I saw how this could be done, this really did feel like the right approach to me. |
Darn, sorry again jedwards . Since I made the mistake once now your name is coming up in the auto-complete in this thread. I'll try to be more careful! |
Support the creation of multiple user_nl files for a component In CESM, we are extending CISM to allow multiple user_nl_cism* files: an overall user_nl_cism, plus a separate file for each ice sheet (user_nl_cism_ais, user_nl_cism_gris, etc.). This PR adds support for staging multiple user_nl files for a component at case.setup time, via an optional function that can be defined in the component's buildnml. See https://github.com/ESCOMP/CISM-wrapper/blob/23b9a83c81db4a952f889496f5ca1dbe62f91178/cime_config/buildnml#L66-L78 for the definition of this function for CISM. To support this change, I have rewritten the CIME functionality that determines if a given function is present in a python module so that it is more robust and covered by unit tests. Test suite: - scripts_regression_tests on izumi: most tests pass; for failures, there were similar failures in a run from an unmodified copy of cime6.0.3 (some of the same tests failed, though there were a few differences that appeared to be random system issues) - manual testing of the changes to `run_sub_or_cmd`, ensuring that the `file_contains_python_function` logic is working correctly - extensive manual testing of the case_setup changes in this PR together with https://github.com/ESCOMP/CISM-wrapper/tree/multiple_user_nl_cisms/ – both with and without the new `get_user_nl_list` function Test baseline: N/A Test namelist changes: none Test status: bit for bit Fixes #3999 User interface changes?: Allows components to define a `get_user_nl_list` function in their buildnml file, which supports the creation of multiple user_nl files for the component Update gh-pages html (Y/N)?: N
CISM in CESM will soon support running multiple ice sheets in the same case. Each of these ice sheets will have its own config file (similar to a namelist file), with different settings. Therefore, in a run containing Greenland and Antarctica (for example) we need a way for users to be able to separately specify settings in the
user_nl_cism
file that apply (1) to both ice sheets; (2) just Greenland; (3) just Antarctica.Based on discussion here ESCOMP/CISM-wrapper#50 the user_nl_cism file will look like this:
I propose adding a function to CIME (in
user_mod_support.py
) to parse a file like this, outputting a new file that just contains the requested subset of settings. In CISM's buildnml, we would then call this function three times:First, we'd request a file that just contains settings that apply to all ice sheets (this is needed to create the top-level
cism_in
file):separate_user_nl(..., section=None)
, which would produce:Then we would call
separate_user_nl(..., section='gris')
, which would produce:Finally, we would call
separate_user_nl(..., section='ais')
, which would produce:(Note: It's intentional that the settings that apply to all ice sheets appear in all cases. This section can contain a mix of settings that (a) go in
cism_in
, which contains non-icesheet-specific settings, and (b) go in allcism.config
files – i.e., ice sheet-specific settings, but for which the user wants the same setting to apply to all ice sheets.)I hope to implement this in the next week or so. I am opening this to gather any feedback on:
Are there any objections to putting this functionality in CIME? I could put it in CISM, but I prefer putting it in CIME for two reasons: (i) I would like to cover this functionality with unit tests; putting it in CISM would require me to build up a new python unit testing infrastructure in CISM. (ii) Maybe this could be useful for other components for some related or different purpose.
Are there any objections to the format of the
user_nl
file proposed above? We are not completely tied to this format, though we (@whlipscomb, @mvertens and I) prefer it over other options we considered (see Determine how to handle user_nl_cism for multiple ice sheets ESCOMP/CISM-wrapper#50).The text was updated successfully, but these errors were encountered: