ESMCI / cime

Common Infrastructure for Modeling the Earth
http://esmci.github.io/cime
Other
162 stars 208 forks source link

Want ability to distinguish compset aliases that should only be used for testing #533

Closed billsacks closed 7 years ago

billsacks commented 8 years ago

CSEG - and particularly the CAM group ( @cacraigucar and others) - have asked for this feature. I am documenting it as an issue based on today's CSEG meeting.

Basically, we want a compset attribute that distinguishes whether a compset is scientifically supported / functionally supported / only available for testing.

Earlier idea was to have a test_alias (rather than alias for this).

But idea from today's discussion is to have a separate support_level attribute. Possible values are:

Then, when you ask for a list of possible compsets, you'd by default only get a list of scientifically supported and functionally supported... but you could ask to see testing-only as well. (And I think the discussion is that you'd need to explicitly ask if you want to create a case with one of these testing-only compsets using create_newcase.)

It was also noted that scientific support is tied to grid. Maybe need an attribute describing that.

Tagging @mvertens @jedwards4b @mnlevy1981 , who are the ones who I remember contributing most to this discussion. Also Sheri Mickelson, but I can't find her github name.

rljacob commented 8 years ago

Do CSEG members want this for themselves or they think this would be a useful feature for other users?

billsacks commented 8 years ago

For ourselves: We want to substantially reduce the number of compsets visible to users, while still maintaining a wider range of configurations for our system test suites.

rljacob commented 8 years ago

The poor compset alias already has to do a lot in a few characters. Maybe a separate config_components.xml file just for the test compsets?

jedwards4b commented 8 years ago

We aren't adding anything to the alias - rather the proposal is to create a new field Currently we have

  <compset>
    <alias>B1850Cw</alias>
    <lname>1850_CAM60%WTSM_CLM50%BGC_CICE_POP2%ECO_MOSART_SGLC_WW3</lname>
  </compset>
jedwards4b commented 8 years ago

The proposal is

    <alias>B1850Cw</alias>
    <support_level> scientific </support_level>
    <lname>1850_CAM60%WTSM_CLM50%BGC_CICE_POP2%ECO_MOSART_SGLC_WW3</lname>
  </compset>```
cacraigucar commented 8 years ago

This is becoming important. A user on the bulletin board posted a question. During his discussion, as an aside, he said he was using the Bi1850 compset. This is a compset which is for testing isotopes only and shouldn't be used for science runs currently. This user naively used it as it seemed to fit his requirements the best (without understanding the subtleties of all the %WISO fields).

rljacob commented 8 years ago

Unless CIME throws an error when someone tries to use an unapproved configuration, there's no way to prevent this. People usually ignore warning messages and will always use the model in ways unintended. I think the solution is better provenance and user's can't get help unless they make their provenance available. The "support level" will quickly become as out-of-date as the old "README.science_support".

cacraigucar commented 7 years ago

Since I've not seen any comments on this in a while, I'm not sure this is actively being worked or not. If it's not, I would like to see a minimal implementation done before a "function freeze".

From my narrow viewpoint, as a minimal implementation, I would like to prevent users from accidentally using a compset for a science run which exists just for testing purposes. We could just have support level added for test compsets , for instance

testing

We could then introduce a flag like -test_compset which would need to be set to run a compset with a support_level of testing. Otherwise, CIME would throw an error and exit.

gold2718 commented 7 years ago

I like the --test-compset flag idea for create_newcase. This flag could be passed from create_test.

rljacob commented 7 years ago

If "support_level" has only 2 possible values, its better to make a logical "TRUE"

gold2718 commented 7 years ago

I'm pretty sure we need more than two. Bill's original post on this issue mentions three essential levels (at least for CESM).

jedwards4b commented 7 years ago

As a response to this issue I added a field:

  <entry id="SCIENCE_SUPPORT">
    <type>char</type>
    <valid_values>on, off</valid_values>
    <default_value>off</default_value>
    <group>case_def</group>
    <file>env_case.xml</file>
    <desc>If set to off, this component set/ grid specification is not scientifically supported.                      
    If set to on, this component set/ grid specification is scientifically supported</desc>
  </entry>

However no one has taken the initiative to use it. If binary is enough this should be changed to a boolean, otherwise we can change the valid_values field. I don't that think there is any consensus yet on how this is to be used.

mvertens commented 7 years ago

I think that all compset definitions should have the following additional entries that could be used to then fill in the SCIENCE_SUPPORT entry in $CASEROOT. Please provide feedback on possible valid values as well as proposed schema below.

<compset>
   <lname>...</lname>
   <alias>...</alias>
   <science_support>
        <valid_values>fully_supported, testing_only, no_testing</valid_values>
        <values>
            <value>no_testing</value>
            <value grid="f09_g16">fully_supported</value>
            <value grid="f19_g16">testing_only</value>
             .....
         <values>
    </science_support>
</entry>
jedwards4b commented 7 years ago

I don't think that this needs to go into all compsets. The default would be no_testing and compsets without this field would get that setting. (Perhaps it should be unsupported rather than no testing)

gold2718 commented 7 years ago

I don't think these are super clear. I like the original tag with possible values: science_support internal_testing unsupported

How many "unsupported" compsets are going to be in the release code? If it is a lot, then I agree that we can leave the tag off with a default of "unsupported".

mvertens commented 7 years ago

I do not agree. If you do not explicitly set it then you don't think about it and it drops out of the system. Insisting at least one value would make sure that new compsets that were added would think of the support level.

On Thu, Jan 12, 2017 at 1:02 PM, jedwards4b notifications@github.com wrote:

I don't think that this needs to go into all compsets. The default would be no_testing and compsets without this field would get that setting. (Perhaps it should be unsupported rather than no testing)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ESMCI/cime/issues/533#issuecomment-272267515, or mute the thread https://github.com/notifications/unsubscribe-auth/AHlxE3s-fbhJO-CovjsreHGNWCg_Zqf3ks5rRobogaJpZM4J6-Ib .

gold2718 commented 7 years ago

But does the whole definition go in the tag or would the compset just have an entry like <support_level> science_support </support_level> or <support_level> unsupported </support_level>?

billsacks commented 7 years ago

I don't have strong feelings on this, but just want to comment on the "valid_values": Is there a way that we can avoid needing to list this repeatedly for every compset? My main concern is what will happen if we add a new valid value: This will either require updating all components to remain in sync, or (more likely) will result in components getting out of sync in this respect, making it hard to determine what the master list of valid values really is.

mvertens commented 7 years ago

As Jim pointed out this would go into the valid values. So this would take care of your concern.

On Thu, Jan 12, 2017 at 1:23 PM, Bill Sacks notifications@github.com wrote:

I don't have strong feelings on this, but just want to comment on the "valid_values": Is there a way that we can avoid needing to list this repeatedly for every compset? My main concern is what will happen if we add a new valid value: This will either require updating all components to remain in sync, or (more likely) will result in components getting out of sync in this respect, making it hard to determine what the master list of valid values really is.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ESMCI/cime/issues/533#issuecomment-272272902, or mute the thread https://github.com/notifications/unsubscribe-auth/AHlxE6c4DC3IGXfzuINUCBmbAlZZ-eFBks5rRoumgaJpZM4J6-Ib .

jedwards4b commented 7 years ago

Yes the definition (including valid_values) is in driver config_component.xml But each compset may have different values depending on the grid, that's why there are multiple values defined.

rljacob commented 7 years ago

So how many combinations of the arguments to create_newcase might be a "test-only" config? Just the compset? compset+resolution? compset+resolution+machine+compiler?

jedwards4b commented 7 years ago

In my opinion only compset and grid.

jedwards4b commented 7 years ago

oh - And I should say that I plan to enforce the combination using an xml schema file so that it cannot grow unbounded. That is if we decide the only attribute is grid then I will enforce that in xml, if we decide also to include other fields then they will also be added to the schema.

gold2718 commented 7 years ago

One issue with making sure that all 'unsupported' configurations are documented is that most possible combinations of compset and grid are not supported. Are we including 'unsupported' tags for all of those?

jedwards4b commented 7 years ago

That is why I suggested that this field does not need to be added for every possible combination, if it is not present then the default setting of unsupported is used. This also covers user defined compsets.

rljacob commented 7 years ago

For a long time we've had a system where the compset and grid listings are separate. Now we are introducing the need to combine them so that "valid" can be determined. How does that change the process of adding a grid or compset?

gold2718 commented 7 years ago

@jedwards4b, I agree which is why I'm responding to the call @mvertens made about always having a tag. The only time that makes sense to me is if we have a compset that is completely unsupported (e.g., for all resolutions). Why would we leave such compsets in the release? Do we have any? I still think we need an 'unsupported' level, I'm just not sure how (or if) to show that in the XML file.

@rljacob, The listings are separate but only some combinations are tested and/or scientifically supported. I don't see any conflict here, could you elaborate?

rljacob commented 7 years ago

Now we need a third thing that indicates which combinations of grid and compset are supported. Where does that live? How does that change the process of adding grids and compsets that are supported?

I agree this is much simpler if just one thing, the compset, determined the supported/not-supported behavior.

jedwards4b commented 7 years ago

@rljacob This does not impose any new restrictions on create_newcase, it would only be used to indicate if a particular combination is supported.

<compset>
   <lname>...</lname>
   <alias>...</alias>
  <science_support>
       <values>
            <value grid="f09_g16">fully_supported</value>
            <value grid="f19_g16">testing_only</value>
         <values>
 </science_support>
</compset>

says that this compset is fully supported on the f09_g16 grid, tested on the f19_g16 grid and all other grids would get an unsupported flag.

mvertens commented 7 years ago

That is not the case for CESM. We will only be providing scientific support for B1850 for f09_g16.

On Thu, Jan 12, 2017 at 1:52 PM, Robert Jacob notifications@github.com wrote:

Now we need a third thing that indicates which combinations of grid and compset are supported. Where does that live? How does that change the process of adding grids and compsets that are supported?

I agree this is much simpler if just one thing, the compset, determined the supported/not-supported behavior.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ESMCI/cime/issues/533#issuecomment-272279976, or mute the thread https://github.com/notifications/unsubscribe-auth/AHlxE44az47xj7ojSPM8Z0423zEjk-Qvks5rRpKMgaJpZM4J6-Ib .

rljacob commented 7 years ago

@billsacks and @cacraigucar were asking for changes in some of the tools behavior based on this new attribute. We should probably spend more time thinking about what we want users to see. That will help with the implementation.

rljacob commented 7 years ago

@mvertens you're not supporting F-cases? I-cases?

gold2718 commented 7 years ago

@jedwards4b , is a slightly simpler syntax possible? Something like:

<compset>
   <lname>...</lname>
   <alias>...</alias>
  <science_support>
        <value grid="f09_g16">fully_supported</value>
        <value grid="f19_g16">testing_only</value>
 </science_support>
</compset>
jedwards4b commented 7 years ago

I don't see any technical incongruities between this proposal and the original request. F-cases and I-cases support level would be defined in the component config_compsets.xml file

jedwards4b commented 7 years ago

We have thus far used

<values>
  <value>
  <value>
</values>

The <values> </values> is used to indicate that the children should be considered as a group - although this field is not strictly required it goes toward consistency with the way we have used this in other xml files.

mvertens commented 7 years ago

Scientific support involves also producing model output data. Each group (i.e. F and I, etc) will be defining their own scientific supported compsets/grids. I was just giving a B compset example.

On Thu, Jan 12, 2017 at 2:01 PM, Robert Jacob notifications@github.com wrote:

@mvertens https://github.com/mvertens you're not supporting F-cases? I-cases?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ESMCI/cime/issues/533#issuecomment-272282193, or mute the thread https://github.com/notifications/unsubscribe-auth/AHlxE-s5DI0X7Aov5cqKwvwY_XXr93JYks5rRpSbgaJpZM4J6-Ib .

gold2718 commented 7 years ago

Right because those were values (e.g., namelist values). These are support levels so another level is not needed. Maybe this would be more clear:

<compset>
   <lname>...</lname>
   <alias>...</alias>
  <science_support>
        <support_level grid="f09_g16">fully_supported</support_level>
        <support_level grid="f19_g16">testing_only</support_level>
 </science_support>
</compset>
cacraigucar commented 7 years ago

Here are my comments on the thread so far:

jedwards4b commented 7 years ago

@gold2718 This would be okay - I think maybe (as yet unconfirmed) I can reuse some existing code if I use values, but I can live with this.

@cacraigucar prevent is a strong word, how about discourages. Unsupported cases require a --force flag to create_newcase.

cacraigucar commented 7 years ago

@jedwards4b My wording should have been "prevent them from inadvertently making". How about instead of --force we have the flag be a little more descriptive like --run_unsupported?

gold2718 commented 7 years ago

I like --include-unsupported

jedwards4b commented 7 years ago

@gold2718 I assume you are talking about the manage_case utility and not create_newcase?

gold2718 commented 7 years ago

Why not create_newcase? With my version, the user picture would be that create_newcase does not look at unsupported compset/grid combinations. The --include-unsupported flag would tell create_newcase to look at those as well. I know what happens under the hood is different but this gives users a consistent picture when using our mainline scripts.

jedwards4b commented 7 years ago

Honestly its too big and long and hard to spell - I like --force

ekluzek commented 7 years ago

Being too big and hard to spell is a feature to dissuade someone from running something unsupported. "force" seems pretty unclear to me as far as what is being forced.

Erik Kluzek, (CGD at NCAR)

National Center for Atmospheric Research

Boulder CO,

(off) (303)497-1326 (fax) (303)497-1348

(skype) ekluzek (cell) (303)859-0183

Pronouns: he/his/him

------------------ Home page ------------------------

!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

On Thu, Jan 12, 2017 at 2:54 PM, jedwards4b notifications@github.com wrote:

Honestly its too big and long and hard to spell - I like --force

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ESMCI/cime/issues/533#issuecomment-272296022, or mute the thread https://github.com/notifications/unsubscribe-auth/ALAswGuBJQVefwlN_3agvPuANY8vghvtks5rRqEzgaJpZM4J6-Ib .

gold2718 commented 7 years ago

We don't want to make it easy :) --force is usually used to un-jam something in a broken state.

cacraigucar commented 7 years ago

I agree that --force is too ambiguous. It is an option that you "try" when "something" isn't working in the hopes it fixes your problem.

mvertens commented 7 years ago

I vote for --include-unsupported also.

rljacob commented 7 years ago

"--force" for create_newcase. You're forcing the new case to be created despite any warnings. "--include-unsupported" makes sense for manage_case

billsacks commented 7 years ago

I like @cacraigucar 's suggestion of --run-unsupported, and like the second part of @rljacob 's suggestion as well. There's a nice parallel here:

manage_case has the --include-unsupported option

create_newcase has the --run-unsupported option

I feel --force is too ambiguous: If you get a few errors in your initial try of create_newcase, then what exactly is it that you're trying to force?

Unless we truly want to go with @rljacob 's suggestion of:

You're forcing the new case to be created despite any warnings

i.e., truly want the --force flag to force the case to be created despite any warnings (case already exists, grid incompatible with compset, etc.)