Skip to content
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

Rework master_list* files etc. #2087

Merged
merged 16 commits into from
Sep 19, 2023
Merged

Conversation

samsrabin
Copy link
Collaborator

@samsrabin samsrabin commented Aug 3, 2023

Description of changes

  • Renames master_list*.rst files to history_fields*.rst.
  • Updates those files with latest output lists and descriptions.
  • Removes line numbers from those files.
  • Renames various variables, changes comments, etc. to avoid use of "master" (in references to these files only).

Will unblock #2074.

Specific notes

Contributors other than yourself, if any: None

CTSM Issues Fixed:
Resolves #2083.
Some work on #1127

Are answers expected to change (and if so in what way)? No

Any User Interface Changes (namelist or namelist defaults changes)? Yes: hist_master_list_file parameter changed to hist_fields_list_file.

Testing performed, if any: Running with hist_fields_list_file = .true. produces history_fields_nofates.rst, which looks fine.

@samsrabin samsrabin added tag: simple bfb documentation additions or edits to user-facing documentation PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete labels Aug 3, 2023
@samsrabin samsrabin self-assigned this Aug 3, 2023
@samsrabin samsrabin requested a review from ekluzek August 7, 2023 16:34
@samsrabin samsrabin added the blocker another issue/PR depends on this one label Aug 10, 2023
@samsrabin
Copy link
Collaborator Author

@ekluzek What do you think of adding this to your upcoming simple bit-for-bit tag? It'd be nice to get it in as it's blocking #2074.

@ekluzek
Copy link
Collaborator

ekluzek commented Aug 10, 2023

@samsrabin yes, I'll add it to the list.

@samsrabin samsrabin removed the PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete label Aug 11, 2023
@samsrabin
Copy link
Collaborator Author

samsrabin commented Aug 11, 2023

  • Resolve issue where history field list file is saved without rst extension.

@samsrabin samsrabin added the PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete label Aug 11, 2023
@samsrabin samsrabin requested a review from ekluzek August 11, 2023 23:12
@samsrabin
Copy link
Collaborator Author

@ekluzek I'm requesting a re-review because I discovered some more places to change variable names and references. I also added a testmod SaveHistFieldList.

Finally, I updated the .rst files that we ship, using the outputs of the following tests:

SMS_Ld1.f10_f10_mg37.I1850Clm50BgcCrop.cheyenne_intel.clm-SaveHistFieldList
SMS_Ld1.f10_f10_mg37.I2000Clm50FatesCru.cheyenne_intel.clm-SaveHistFieldList

These tests use the default output variables (i.e., without specifying any additional outputs in user_nl_clm). The .rst files thus indicate "Active?" with T/F based on the default list. I felt this would be the most useful for people referring to those files.

However, it differs from the description @slevis-lmwg wrote in README_master_list_files (original text below; diff with my updated version here):

2021/9/8 slevis

The files master_list_nofates.rst and master_list_fates.rst each contain a table of the history fields, active and inactive, available in the CTSM cases that get generated by these tests:
ERP_P36x2_D_Ld3.f10_f10_mg37.I1850Clm50BgcCrop.cheyenne_gnu.clm-extra_outputs
ERS_Ld9.f10_f10_mg37.I2000Clm50FatesCru.cheyenne_intel.clm-FatesColdDefCH4

To reproduce these .rst files, run the above tests and the files will appear in the corresponding run directories.

What do y'all think of my change to indicate "Active?" based on the default settings?

@slevis-lmwg
Copy link
Contributor

What do y'all think of my change to indicate "Active?" based on the default settings?

If I'm understanding correctly that this will indicate active/inactive for each field, yet all the fields will still get listed, then I like that.

@samsrabin
Copy link
Collaborator Author

Yep, that's right!

@samsrabin
Copy link
Collaborator Author

samsrabin commented Aug 21, 2023

  • Add vertical dimension (e.g., nlevsoi) as a column to the .rst files. If none, use - to match ncdump convention.
  • Merge in changes from history write optimization #2115 once that's complete.
  • Do a final update to the .rst files.

@samsrabin samsrabin removed the PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete label Aug 21, 2023
@billsacks
Copy link
Member

@samsrabin - thanks for your recent commit adding the level dimension to this table!

Change order of history fields to improve performance on derecho

Instead of just ordering history fields alphabetically, order them first
by the name of their level dimension (with fields without a level
dimension appearing first), then alphabetically within a given level
dimension. This changed ordering gives a significant performance
improvement especially noticeable on lustre file systems such as on
derecho.

# Conflicts:
#	src/main/histFileMod.F90
@samsrabin samsrabin added the PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete label Aug 23, 2023
Copy link
Collaborator

@ekluzek ekluzek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great. The change to remove the field number from the files will really help prevent git conflicts in those files. I also like your removal of master for the field lists. You probably didn't even know, but that is work towards #1127 which is really awesome. There are some technical terms in use that have negative historical connotations and we should remove them from our use. So it's good to be making some progress in that regard.

@ekluzek
Copy link
Collaborator

ekluzek commented Sep 18, 2023

Oh, something we should check if this interfere's with some of @johnpaulalex PR's that are still outstanding. So I'll try a merge and see what I see...

@ekluzek
Copy link
Collaborator

ekluzek commented Sep 18, 2023

It turns out @samsrabin had already checked @johnpaulalex PR's, but I ended up rechecking and came to the same conclusion. There's one PR that has a conflict, but it won't be hard to resolve. But, it might have been good for me to check to know what to do to resolve it.

samsrabin added a commit that referenced this pull request Sep 19, 2023
* Add system and unit tests for making fsurdat with all crops everywhere (#2081)
* Rework master_list* files etc. (#2087)
* Fixes to methane Tech Note (#2091)
* Add is_doy_in_interval() function (#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (#2079)
* Rework master_list_(no)?fates.rst? (#2083)
* conda run -n can fail if a conda environment is already active (#2109)
* conda fails to load for SystemTests (#2111)
@samsrabin samsrabin merged commit 4eb3eaa into ESCOMP:master Sep 19, 2023
2 checks passed
samsrabin added a commit to samsrabin/CTSM that referenced this pull request Sep 19, 2023
b4b changes to Python scripts, history lists, tech note, and clm_time_manager.

* Add system and unit tests for making fsurdat with all crops everywhere (ESCOMP#2081)
* Rework master_list* files etc. (ESCOMP#2087)
* Fixes to methane Tech Note (ESCOMP#2091)
* Add is_doy_in_interval() function (ESCOMP#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (ESCOMP#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (ESCOMP#2079)
* Rework master_list_(no)?fates.rst? (ESCOMP#2083)
* conda run -n can fail if a conda environment is already active (ESCOMP#2109)
* conda fails to load for SystemTests (ESCOMP#2111)
samsrabin added a commit to samsrabin/CTSM that referenced this pull request Sep 20, 2023
b4b changes to Python scripts, history lists, tech note, and clm_time_manager.

* Add system and unit tests for making fsurdat with all crops everywhere (ESCOMP#2081)
* Rework master_list* files etc. (ESCOMP#2087)
* Fixes to methane Tech Note (ESCOMP#2091)
* Add is_doy_in_interval() function (ESCOMP#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (ESCOMP#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (ESCOMP#2079)
* Rework master_list_(no)?fates.rst? (ESCOMP#2083)
* conda run -n can fail if a conda environment is already active (ESCOMP#2109)
* conda fails to load for SystemTests (ESCOMP#2111)
samsrabin added a commit to samsrabin/CTSM that referenced this pull request Sep 21, 2023
b4b changes to Python scripts, history lists, tech note, and clm_time_manager.

* Add system and unit tests for making fsurdat with all crops everywhere (ESCOMP#2081)
* Rework master_list* files etc. (ESCOMP#2087)
* Fixes to methane Tech Note (ESCOMP#2091)
* Add is_doy_in_interval() function (ESCOMP#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (ESCOMP#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (ESCOMP#2079)
* Rework master_list_(no)?fates.rst? (ESCOMP#2083)
* conda run -n can fail if a conda environment is already active (ESCOMP#2109)
* conda fails to load for SystemTests (ESCOMP#2111)
samsrabin added a commit to samsrabin/CTSM that referenced this pull request Sep 27, 2023
b4b changes to Python scripts, history lists, tech note, and clm_time_manager.

* Add system and unit tests for making fsurdat with all crops everywhere (ESCOMP#2081)
* Rework master_list* files etc. (ESCOMP#2087)
* Fixes to methane Tech Note (ESCOMP#2091)
* Add is_doy_in_interval() function (ESCOMP#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (ESCOMP#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (ESCOMP#2079)
* Rework master_list_(no)?fates.rst? (ESCOMP#2083)
* conda run -n can fail if a conda environment is already active (ESCOMP#2109)
* conda fails to load for SystemTests (ESCOMP#2111)
samsrabin added a commit to samsrabin/CTSM that referenced this pull request Oct 2, 2023
b4b changes to Python scripts, history lists, tech note, and clm_time_manager.

* Add system and unit tests for making fsurdat with all crops everywhere (ESCOMP#2081)
* Rework master_list* files etc. (ESCOMP#2087)
* Fixes to methane Tech Note (ESCOMP#2091)
* Add is_doy_in_interval() function (ESCOMP#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (ESCOMP#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (ESCOMP#2079)
* Rework master_list_(no)?fates.rst? (ESCOMP#2083)
* conda run -n can fail if a conda environment is already active (ESCOMP#2109)
* conda fails to load for SystemTests (ESCOMP#2111)
samsrabin added a commit to samsrabin/CTSM that referenced this pull request Oct 3, 2023
b4b changes to Python scripts, history lists, tech note, and clm_time_manager.

* Add system and unit tests for making fsurdat with all crops everywhere (ESCOMP#2081)
* Rework master_list* files etc. (ESCOMP#2087)
* Fixes to methane Tech Note (ESCOMP#2091)
* Add is_doy_in_interval() function (ESCOMP#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (ESCOMP#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (ESCOMP#2079)
* Rework master_list_(no)?fates.rst? (ESCOMP#2083)
* conda run -n can fail if a conda environment is already active (ESCOMP#2109)
* conda fails to load for SystemTests (ESCOMP#2111)
samsrabin added a commit to samsrabin/CTSM that referenced this pull request Oct 4, 2023
b4b changes to Python scripts, history lists, tech note, and clm_time_manager.

* Add system and unit tests for making fsurdat with all crops everywhere (ESCOMP#2081)
* Rework master_list* files etc. (ESCOMP#2087)
* Fixes to methane Tech Note (ESCOMP#2091)
* Add is_doy_in_interval() function (ESCOMP#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (ESCOMP#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (ESCOMP#2079)
* Rework master_list_(no)?fates.rst? (ESCOMP#2083)
* conda run -n can fail if a conda environment is already active (ESCOMP#2109)
* conda fails to load for SystemTests (ESCOMP#2111)
samsrabin added a commit to samsrabin/CTSM that referenced this pull request Oct 5, 2023
b4b changes to Python scripts, history lists, tech note, and clm_time_manager.

* Add system and unit tests for making fsurdat with all crops everywhere (ESCOMP#2081)
* Rework master_list* files etc. (ESCOMP#2087)
* Fixes to methane Tech Note (ESCOMP#2091)
* Add is_doy_in_interval() function (ESCOMP#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (ESCOMP#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (ESCOMP#2079)
* Rework master_list_(no)?fates.rst? (ESCOMP#2083)
* conda run -n can fail if a conda environment is already active (ESCOMP#2109)
* conda fails to load for SystemTests (ESCOMP#2111)
samsrabin added a commit to samsrabin/CTSM that referenced this pull request Dec 23, 2023
b4b changes to Python scripts, history lists, tech note, and clm_time_manager.

* Add system and unit tests for making fsurdat with all crops everywhere (ESCOMP#2081)
* Rework master_list* files etc. (ESCOMP#2087)
* Fixes to methane Tech Note (ESCOMP#2091)
* Add is_doy_in_interval() function (ESCOMP#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (ESCOMP#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (ESCOMP#2079)
* Rework master_list_(no)?fates.rst? (ESCOMP#2083)
* conda run -n can fail if a conda environment is already active (ESCOMP#2109)
* conda fails to load for SystemTests (ESCOMP#2111)

# Conflicts:
#	src/biogeochem/CNBalanceCheckMod.F90
#	src/biogeochem/CNCIsoFluxMod.F90
#	src/biogeochem/CNDriverMod.F90
#	src/biogeochem/CNPhenologyMod.F90
#	src/biogeochem/CNProductsMod.F90
#	src/biogeochem/CNVegCarbonFluxType.F90
#	src/biogeochem/CNVegNitrogenFluxType.F90
#	src/biogeochem/EDBGCDynMod.F90
#	src/main/clm_initializeMod.F90
#	src/main/controlMod.F90
#	src/soilbiogeochem/SoilBiogeochemDecompCascadeBGCMod.F90
samsrabin added a commit to samsrabin/CTSM that referenced this pull request Dec 23, 2023
b4b changes to Python scripts, history lists, tech note, and clm_time_manager.

* Add system and unit tests for making fsurdat with all crops everywhere (ESCOMP#2081)
* Rework master_list* files etc. (ESCOMP#2087)
* Fixes to methane Tech Note (ESCOMP#2091)
* Add is_doy_in_interval() function (ESCOMP#2158)
* Avoid using subprocess.run() in FSURDATMODIFYCTSM (ESCOMP#2125)

Closes issues:
* Add unit test for making fsurdat with all crops everywhere (ESCOMP#2079)
* Rework master_list_(no)?fates.rst? (ESCOMP#2083)
* conda run -n can fail if a conda environment is already active (ESCOMP#2109)
* conda fails to load for SystemTests (ESCOMP#2111)
@samsrabin samsrabin added simple bfb bit-for-bit labels Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bfb bit-for-bit blocker another issue/PR depends on this one documentation additions or edits to user-facing documentation PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rework master_list_(no)?fates.rst?
4 participants