Recent cime versions let you create a case with a non-existent compset

[billsacks]

If you try to create a case with a compset alias, where that alias isn't actually defined anywhere, cime should (and I'm pretty sure did, in the past) abort with an error saying that that alias cannot be found. But now the scripts happily proceed, treating the alias as the date specifier and using all stub components: e.g., from a standalone cime checkout, at the current master:

./create_newcase --compset I2000Ctsm50NwpBgcCropGswpGs --res f10_f10_musgs --case $scratch/test_err_0722a --run-unsupported

This happily creates the case; according to README.case, the generated compset long name is:

I2000Ctsm50NwpBgcCropGswpGs_SATM_SLND_SICE_SOCN_SROF_SGLC_SWAV_SIAC_SESP

My initial hunch is that this problem came in when code was added to allow stub components to be absent in the specified compset name (#3068 from @gold2718 ), but I haven't looked closely to confirm this hunch.

(This problem was first picked up by @slevisconsulting.)

[gold2718]

This feels like something that should have a regression test. @jgfouca, can you point me to an example of how to set up an expected FAIL such as this?

[jgfouca]

@gold2718 , see K_TestCimeCase in scripts_regression_tests.

[gold2718]

@jgfouca,

Thanks. It seems for this test, do I just run create_newcase and make sure there is no case directory? Something like:

        self.assertFalse(os.path.isdir(casedir),
                         msg="create_newcase should fail for compset, NonexistentCompset)

Could this be added to J_TestCreateNewcase?

Unrelated style question: Is there a reason for this type of test:

        self.assertEqual(type(MACHINE.get_value("MAX_TASKS_PER_NODE")), int)

instead of:

        self.assertTrue(isinstance(MACHINE.get_value("MAX_TASKS_PER_NODE"), int))

given that isinstance is preferred over using type?

[jgfouca]

@gold2718 , I think you want to run create_newcase and confirm that it fails with a non-zero return code if the compset is nonsense. Use run_cmd_assert_result with a non-zero expected_stat to confirm this. J_TestCreateNewcase would be a fine class to host this test.

Unrelated style question. Is there a reason for this type of test: ...

None that I can think of.

[billsacks]

@gold2718 maybe you've already thought of something more clever than what I can think of, but just to share my initial thoughts: I'm wondering if it's going to now be hard to distinguish between an alias and a long name, given the flexibility in specifying a long name. The first thing that comes to mind is a rule: aliases must not have any underscore in them; long names must have at least one underscore. I'm not sure if that will work with all of our current aliases (in both CESM and E3SM). I also realize it may be more restrictive than absolutely necessary for long names, but maybe that's okay? (i.e., you need to specify at least two things explicitly.)

As for the assert style question: I tend to prefer assertEqual (or variants) over assertTrue / assertFalse, because the former gives more information if the test fails (giving both values, rather than just saying that it expected true and got false). But that's not a strong preference, in cases where it is cleaner to use assertTrue / assertFalse.

[gold2718]

@billsacks, E3SM seems to allow underscores in their aliases (cf TG_MLI in components/mpas-albany-landice/cime_config/config_compsets.xml). On the other hand, long names all seem to have an underscore in both models due to my least-favorite feature, the time code (or whatever the first field is called) so I will check for that. This still has the hole when someone mistypes an alias with an underscore (e.g., TG_ML1) -- it will generate a really confusing error message:

Did not find an alias or longname compset match for TG_ML1 
ERROR: Unknown model type, ML1

Still better than the current situation (which I keep tripping over)?

[billsacks]

Sounds good. It seems like the logic will be trickier if you need to allow for the possibility that a compset with an underscore could either be an alias or a long name, but if you have looked at it and it's not too bad, then we can go with that rather than forcing E3SM to change. On the other hand, if it keeps things significantly cleaner to impose this new rule, then it may be worth checking in with the E3SM land ice group and seeing if they're willing to change this (I'm happy to be the one to reach out to them if you'd like).

If we do allow for underscores in aliases, I suppose that error message could be extended to something like, "Unknown model type, ML1 (note: the given compset name is being interpreted as a long name; if you intended to use a short compset alias, double-check that you are using an existing alias)", but I don't feel strongly about whether that's worth doing.

[gold2718]

@billsacks, If you could reach out, that would be great. @jgfouca, do you know if underscores in compset aliases was an intentional move for E3SM or just something that leaked in?

[billsacks]

It looks to me like underscores in compset aliases are much more widespread than just the E3SM land ice group: From ./query_config --compsets | grep '_.*:' (setting CIME_MODEL appropriately in each case), I get:

E3SM:

   A_WCYCL1850S_CMIP6   : 1850_CAM5%CMIP6_CLM45%SPBC_MPASSI%SPUNUP_MPASO%SPUNUP_MOSART_SGLC_SWAV
   A_WCYCL20TRS_CMIP6   : 20TR_CAM5%CMIP6_CLM45%SPBC_MPASSI%SPUNUP_MPASO%SPUNUP_MOSART_SGLC_SWAV
   A_WCYCL1950S_CMIP6_LR : 1950_CAM5%CMIP6-LR_CLM45%SPBC_MPASSI%SPUNUP_MPASO%SPUNUP_MOSART_SGLC_SWAV
   A_WCYCL1950S_CMIP6_HR : 1950_CAM5%CMIP6-HR_CLM45%SPBC_MPASSI%SPUNUP_MPASO%SPUNUP_MOSART_SGLC_SWAV
   A_WCYCL1950S_CMIP6_LRtunedHR : 1950_CAM5%CMIP6-LRtunedHR_CLM45%SPBC_MPASSI%SPUNUP_MPASO%SPUNUP_MOSART_SGLC_SWAV
   A_WCYCL2000          : 2000_CAM5%AV1C-L_CLM45%SPBC_MPASSI_MPASO_MOSART_SGLC_SWAV
   A_WCYCL2000S         : 2000_CAM5%AV1C-L_CLM45%SPBC_MPASSI%SPUNUP_MPASO%SPUNUP_MOSART_SGLC_SWAV
   A_WCYCL1850          : 1850_CAM5%AV1C-L_CLM45%SPBC_MPASSI_MPASO_MOSART_SGLC_SWAV
   A_WCYCL1850S         : 1850_CAM5%AV1C-L_CLM45%SPBC_MPASSI%SPUNUP_MPASO%SPUNUP_MOSART_SGLC_SWAV
   A_WCYCL20TR          : 20TR_CAM5%AV1C-L_CLM45%SPBC_MPASSI_MPASO_MOSART_SGLC_SWAV
   A_WCYCL20TRS         : 20TR_CAM5%AV1C-L_CLM45%SPBC_MPASSI%SPUNUP_MPASO%SPUNUP_MOSART_SGLC_SWAV
   A_CRYO               : 2000_CAM5%AV1C-L_CLM45%SPBC_MPASSI_MPASO_MOSART_MALI_SWAV
   A_WCYCL2000_H01A     : 2000_CAM5%AV1C-H01A_CLM45%SPBC_MPASSI_MPASO_MOSART_SGLC_SWAV
   A_WCYCL2000_H01AS    : 2000_CAM5%AV1C-H01A_CLM45%SPBC_MPASSI%SPUNUP_MPASO%SPUNUP_MOSART_SGLC_SWAV
   A_WCYCL1850_H01A     : 1850_CAM5%AV1C-H01A_CLM45%SPBC_MPASSI_MPASO_MOSART_SGLC_SWAV
   A_WCYCL1850_H01AS    : 1850_CAM5%AV1C-H01A_CLM45%SPBC_MPASSI%SPUNUP_MPASO%SPUNUP_MOSART_SGLC_SWAV
   A_WCYCL20TR_H01A     : 20TR_CAM5%AV1C-H01A_CLM45%SPBC_MPASSI_MPASO_MOSART_SGLC_SWAV
   A_WCYCL20TR_H01AS    : 20TR_CAM5%AV1C-H01A_CLM45%SPBC_MPASSI%SPUNUP_MPASO%SPUNUP_MOSART_SGLC_SWAV
   BGCEXP_BCRC_CNPRDCTC_1850 : 1850_CAM5%CMIP6_CLM45%CNPRDCTCBC_MPASSI%BGC_MPASO%OIECOOIDMS_MOSART_SGLC_SWAV_BGC%BCRC
   BGCEXP_BCRC_CNPRDCTC_1850S : 1850_CAM5%CMIP6_CLM45%CNPRDCTCBC_MPASSI%SPUNUP_MPASO%SPUNUP_MOSART_SGLC_SWAV_BGC%BCRC
   BGCEXP_BCRC_CNPRDCTC_20TR : 20TR_CAM5%CMIP6_CLM45%CNPRDCTCBC_MPASSI%BGC_MPASO%OIECOOIDMS_MOSART_SGLC_SWAV_BGC%BCRC
   BGCEXP_BCRC_CNPRDCTC_20TRS : 20TR_CAM5%CMIP6_CLM45%CNPRDCTCBC_MPASSI%SPUNUP_MPASO%SPUNUP_MOSART_SGLC_SWAV_BGC%BCRC
   BGCEXP_BCRD_CNPRDCTC_20TR : 20TR_CAM5%CMIP6_CLM45%CNPRDCTCBC_MPASSI%BGC_MPASO%OIECOOIDMS_MOSART_SGLC_SWAV_BGC%BCRD
   BGCEXP_BCRD_CNPRDCTC_20TRS : 20TR_CAM5%CMIP6_CLM45%CNPRDCTCBC_MPASSI%SPUNUP_MPASO%SPUNUP_MOSART_SGLC_SWAV_BGC%BCRD
   BGCEXP_BDRC_CNPRDCTC_20TR : 20TR_CAM5%CMIP6_CLM45%CNPRDCTCBC_MPASSI%BGC_MPASO%OIECOOIDMS_MOSART_SGLC_SWAV_BGC%BDRC
   BGCEXP_BDRC_CNPRDCTC_20TRS : 20TR_CAM5%CMIP6_CLM45%CNPRDCTCBC_MPASSI%SPUNUP_MPASO%SPUNUP_MOSART_SGLC_SWAV_BGC%BDRC
   BGCEXP_BDRD_CNPRDCTC_20TR : 20TR_CAM5%CMIP6_CLM45%CNPRDCTCBC_MPASSI%BGC_MPASO%OIECOOIDMS_MOSART_SGLC_SWAV_BGC%BDRD
   BGCEXP_BDRD_CNPRDCTC_20TRS : 20TR_CAM5%CMIP6_CLM45%CNPRDCTCBC_MPASSI%SPUNUP_MPASO%SPUNUP_MOSART_SGLC_SWAV_BGC%BDRD
   BGCEXP_BCRC_CNPECACNT_1850 : 1850_CAM5%CMIP6_CLM45%CNPECACNTBC_MPASSI%BGC_MPASO%OIECOOIDMS_MOSART_SGLC_SWAV_BGC%BCRC
   BGCEXP_BCRC_CNPECACNT_20TR : 20TR_CAM5%CMIP6_CLM45%CNPECACNTBC_MPASSI%BGC_MPASO%OIECOOIDMS_MOSART_SGLC_SWAV_BGC%BCRC
   BGCEXP_BCRD_CNPECACNT_20TR : 20TR_CAM5%CMIP6_CLM45%CNPECACNTBC_MPASSI%BGC_MPASO%OIECOOIDMS_MOSART_SGLC_SWAV_BGC%BCRD
   BGCEXP_BDRC_CNPECACNT_20TR : 20TR_CAM5%CMIP6_CLM45%CNPECACNTBC_MPASSI%BGC_MPASO%OIECOOIDMS_MOSART_SGLC_SWAV_BGC%BDRC
   BGCEXP_BDRD_CNPECACNT_20TR : 20TR_CAM5%CMIP6_CLM45%CNPECACNTBC_MPASSI%BGC_MPASO%OIECOOIDMS_MOSART_SGLC_SWAV_BGC%BDRD
   A_WCYCL1850-DIB      : 1850_CAM5%AV1C-L_CLM45%SPBC_MPASSI%DIB_MPASO%IB_MOSART_SGLC_SWAV
   A_WCYCL1850-DIB-ISMF : 1850_CAM5%AV1C-L_CLM45%SPBC_MPASSI%DIB_MPASO%IBISMF_MOSART_SGLC_SWAV
   A_WCYCL1850-DIB_CMIP6 : 1850_CAM5%CMIP6_CLM45%SPBC_MPASSI%DIB_MPASO%IB_MOSART_SGLC_SWAV
   A_WCYCL1850-DIB-ISMF_CMIP6 : 1850_CAM5%CMIP6_CLM45%SPBC_MPASSI%DIB_MPASO%IBISMF_MOSART_SGLC_SWAV
   A_WCYCL1850_v0atm    : 1850_CAM5_CLM45%SP_MPASSI_MPASO_MOSART_SGLC_SWAV
   A_WCYCL2000_v0atm    : 2000_CAM5_CLM45%SP_MPASSI_MPASO_MOSART_SGLC_SWAVi
   A_WCYCL1850S_v0atm   : 1850_CAM5_CLM45%SP_MPASSI%SPUNUP_MPASO%SPUNUP_MOSART_SGLC_SWAV
   A_WCYCL2000S_v0atm   : 2000_CAM5_CLM45%SP_MPASSI%SPUNUP_MPASO%SPUNUP_MOSART_SGLC_SWAVi
   A_BG1850CN           : 1850_CAM5_CLM45%CN_MPASSI_MPASO_MOSART_MALI%SIA_SWAV
   MPAS_LISIO_TEST      : 2000_DATM%NYF_SLND_MPASSI_MPASO_DROF%NYF_MALI%SIA_SWAV
   ADESP_TEST           : 2000_DATM%NYF_SLND_SICE_DOCN%SOMAQP_SROF_SGLC_SWAV_DESP%TEST
   F_ARM97_SCAM5        : AR97_CAM5%SCAM_CLM45%SP_CICE%PRES_DOCN%DOM_SROF_SGLC_SWAV
   F_SCAM5              : SCAM_CAM5%SCAM_CLM45%SP_CICE%PRES_DOCN%DOM_SROF_SGLC_SWAV
   IGCLM45_MLI          : 2000_DATM%QIA_CLM45%SP_SICE_SOCN_MOSART_MALI%SIA_SWAV
   TG_MLI               : 2000_SATM_DLND%SCPL_SICE_SOCN_SROF_MALI_SWAV

CESM:

   ADESP_TEST           : 2000_DATM%NYF_SLND_SICE_DOCN%SOMAQP_SROF_SGLC_SWAV_DESP%TEST
   FHIST_BGC            : HIST_CAM60_CLM50%BGC-CROP_CICE%PRES_DOCN%DOM_MOSART_CISM2%NOEVOLVE_SWAV
   F1850_BDRD           : 1850_CAM60_CLM50%BGC_CICE%PRES_DOCN%DOM_MOSART_CISM2%NOEVOLVE_SWAV_BGC%BDRD
   FHIST_BDRD           : HIST_CAM60_CLM50%BGC-CROP_CICE%PRES_DOCN%DOM_MOSART_CISM2%NOEVOLVE_SWAV_BGC%BDRD
   FHIST_DARTC6         : HIST_CAM60_CLM50%SP_CICE%PRES_DOCN%DOM_SROF_SGLC_SWAV
   FWHIST_BGC           : HIST_CAM60%WCTS_CLM50%BGC-CROP_CICE%PRES_DOCN%DOM_MOSART_CISM2%NOEVOLVE_SWAV
   C_HR                 : 2000_DATM%NYF_SLND_DICE%SSMI_POP2_DROF%NYF_SGLC_SWAV
   C_JRA                : 2000_DATM%JRA_SLND_DICE%SSMI_POP2_DROF%JRA_SGLC_WW3
   C_JRA_HR             : 2000_DATM%JRA_SLND_DICE%SSMI_POP2_DROF%JRA_SGLC_SWAV
   GIAF_HR              : 2000_DATM%IAF_SLND_CICE_POP2_DROF%IAF_SGLC_SWAV
   GIAF_JRA             : 2000_DATM%JRA_SLND_CICE_POP2_DROF%JRA_SGLC_WW3
   GIAF_JRA_HR          : 2000_DATM%JRA_SLND_CICE_POP2_DROF%JRA_SGLC_SWAV
   G1850ECO_CPLHIST     : 1850_DATM%CPLHIST_SLND_CICE_POP2%ECO_DROF%CPLHIST_SGLC_WW3
   G1850ECO_CPLHIST_PHYS_CYCLE : 1850_DATM%CPLHIST_SLND_CICE_POP2%ECO%PHYS-CYCLE_DROF%CPLHIST_SGLC_WW3
   C_WAV                : 2000_DATM%NYF_SLND_DICE%SSMI_POP2_DROF%NYF_SGLC_WW3
   CIAF_WAV             : 2000_DATM%IAF_SLND_DICE%IAF_POP2_DROF%IAF_SGLC_WW3
   G_WAV                : 2000_DATM%NYF_SLND_CICE_POP2_DROF%NYF_SGLC_WW3
   GIAF_WAV             : 2000_DATM%IAF_SLND_CICE_POP2_DROF%IAF_SGLC_WW3
   C_DWAV               : 2000_DATM%NYF_SLND_DICE%SSMI_POP2_DROF%NYF_SGLC_DWAV%CLIMO
   CIAF_DWAV            : 2000_DATM%IAF_SLND_DICE%IAF_POP2_DROF%IAF_SGLC_DWAV%CLIMO
   G_DWAV               : 2000_DATM%NYF_SLND_CICE_POP2_DROF%NYF_SGLC_DWAV%CLIMO
   GIAF_DWAV            : 2000_DATM%IAF_SLND_CICE_POP2_DROF%IAF_SGLC_DWAV%CLIMO

So, while I like the rule that compset aliases shouldn't have an underscore, it looks like that change would require many changes for both models.

[rljacob]

It was not intentional in E3SM. It started with the "A_" aliases and spread from there.

[jedwards4b]

I have a fix for this - it throws an error if all components are stubs - unless you are in a test. It is allowed for tests so that compset S will still work.

[gold2718]

I have a branch that has a test for this bug. Can we do some combination?

The fix I was going to try (but have not gotten around to) is to only allow all stubs if it was found in an alias compset_alias is not None. This seems like it would work and is a lot simpler.

[jedwards4b]

@gold2718 No problem, your solution sounds fine.

[gold2718]

I am running tests now. I just passed the compset_alias into valid_compset and then used that for the allstubs check.

[megranaterogger]

If you try to create a case with a compset alias, where that alias isn't actually defined anywhere, cime should (and I'm pretty sure did, in the past) abort with an error saying that that alias cannot be found. But now the scripts happily proceed, treating the alias as the date specifier and using all stub components: e.g., from a standalone cime checkout, at the current master:

./create_newcase --compset I2000Ctsm50NwpBgcCropGswpGs --res f10_f10_musgs --case $scratch/test_err_0722a --run-unsupported

This happily creates the case; according to README.case, the generated compset long name is:

I2000Ctsm50NwpBgcCropGswpGs_SATM_SLND_SICE_SOCN_SROF_SGLC_SWAV_SIAC_SESP

My initial hunch is that this problem came in when code was added to allow stub components to be absent in the specified compset name (#3068 from @gold2718 ), but I haven't looked closely to confirm this hunch.

(This problem was first picked up by @slevisconsulting

If you try to create a case with a compset alias, where that alias isn't actually defined anywhere, cime should (and I'm pretty sure did, in the past) abort with an error saying that that alias cannot be found. But now the scripts happily proceed, treating the alias as the date specifier and using all stub components: e.g., from a standalone cime checkout, at the current master:

./create_newcase --compset I2000Ctsm50NwpBgcCropGswpGs --res f10_f10_musgs --case $scratch/test_err_0722a --run-unsupported

This happily creates the case; according to README.case, the generated compset long name is:

I2000Ctsm50NwpBgcCropGswpGs_SATM_SLND_SICE_SOCN_SROF_SGLC_SWAV_SIAC_SESP

My initial hunch is that this problem came in when code was added to allow stub components to be absent in the specified compset name (#3068 from @gold2718 ), but I haven't looked closely to confirm this hunch.

(This problem was first picked up by @slevisconsulting.)

Execuse me, is anyone know what kinds of data show i download if i want run B1850 and F1850 compset in my local machine?