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

Delete _FillValue and history from parameter files #2350

Merged
merged 4 commits into from
Feb 8, 2024

Conversation

samsrabin
Copy link
Collaborator

@samsrabin samsrabin commented Feb 7, 2024

Description of changes

Updates parameter files to c240207b. These are the same as c240105 except:

  • Attribute _FillValue has been removed from variables that had it erroneously added in c240105
  • Global attributes history, history_of_appended_files, and latest_git_log have been removed

Specific notes

Contributors other than yourself, if any: None

CTSM Issues Fixed:

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

Any User Interface Changes (namelist or namelist defaults changes)? No

Testing performed, if any:

  • aux_clm OK as of 1949f5c
  • ncdump -v bgc_rf_l1s1 prints the value instead of _

@samsrabin samsrabin added tag: simple bfb bug something is working incorrectly labels Feb 7, 2024
@samsrabin samsrabin self-assigned this Feb 7, 2024
@samsrabin samsrabin added next this should get some attention in the next week or two. Normally each Thursday SE meeting. and removed next this should get some attention in the next week or two. Normally each Thursday SE meeting. labels Feb 7, 2024
@samsrabin
Copy link
Collaborator Author

@wwieder and @olyson: New parameter files are available for you to try:

  • clm45_params.c240207.nc
  • clm50_params.c240207.nc
  • ctsm51_params.c240207.nc
  • ctsm51_ciso_cwd_hr_params.c240207.nc

@olyson
Copy link
Contributor

olyson commented Feb 8, 2024

Thanks, that seems to be working for me.

@samsrabin samsrabin added the PR status: awaiting review Work on this PR is paused while waiting for review. label Feb 8, 2024
@samsrabin
Copy link
Collaborator Author

@ekluzek This is ready for your approval.

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.

Thanks for working on this @samsrabin. This will be nice to get in ASAP, since it's causing trouble for people. I'm also glad that you removed the history attribute as we talked about since it wasn't working in cprnc. history is a standard CF standard variable that some tools also update automatically when a change is made (for example NCO), so it's useful in general, but it had become too long. But, we can add it back in when it makes sense, which as we said will likely be when we start tuning things.

This is of course really simple, since it just changes the parameter files. I'm hitting this as approve so as to not slow it down. But, I do have an important question.

This removes _FillValue that were nan, and _FillValue for scalars both of which make sense. What I'm not so sure about is that it also removes _FillValue for arrays and it's also removing _FillValue's that were consciously set to something. Here's a list of non NaN _FillValues...

- 		allconsl:_FillValue = 0. ;
- 		allconss:_FillValue = 0. ;
- 		arootf:_FillValue = 0. ;
- 		arooti:_FillValue = 0. ;
- 		astemf:_FillValue = 0. ;
- 		bfact:_FillValue = 0. ;
- 		cc_dstem:_FillValue = -999.99 ;
- 		cc_leaf:_FillValue = -999.99 ;
- 		cc_lstem:_FillValue = -999.99 ;
- 		cc_other:_FillValue = -999.99 ;
- 		declfact:_FillValue = 0. ;
- 		fd_pft:_FillValue = -999.99 ;
- 		fleafi:_FillValue = 0. ;
- 		fm_droot:_FillValue = -999.99 ;
- 		fm_dstem:_FillValue = -999.99 ;
- 		fm_leaf:_FillValue = -999.99 ;
- 		fm_lroot:_FillValue = -999.99 ;
- 		fm_lstem:_FillValue = -999.99 ;
- 		fm_other:_FillValue = -999.99 ;
- 		fm_root:_FillValue = -999.99 ;
- 		fsr_pft:_FillValue = -999.99 ;
- 		gddmin:_FillValue = 0. ;
- 		graincn:_FillValue = 0. ;
- 		grnfill:_FillValue = 0. ;
- 		hybgdd:_FillValue = 0. ;
- 		laimx:_FillValue = 0. ;
- 		lfemerg:_FillValue = 0. ;
- 		max_NH_planting_date:_FillValue = 0 ;
- 		max_SH_planting_date:_FillValue = 0 ;
- 		mergetoclmpft:_FillValue = 0 ;
- 		min_NH_planting_date:_FillValue = 0 ;
- 		min_SH_planting_date:_FillValue = 0 ;
- 		min_planting_temp:_FillValue = 1000. ;
- 		mxmat:_FillValue = 0 ;
- 		mxtmp:_FillValue = 0. ;
- 		pftpar20:_FillValue = 9999.9 ;
- 		pftpar28:_FillValue = 9999.9 ;
- 		pftpar29:_FillValue = 1000. ;
- 		pftpar31:_FillValue = 1000. ;
- 		planting_temp:_FillValue = 1000. ;
- 		pprodharv10:_FillValue = 0. ;
- 		variantnames:_FillValue = "" ;
- 		ztopmx:_FillValue = 0. ;

I think those ones likely have some meaning and should probably remain as they were. Other's of the arrays probably should have _FillValue as well, but it would take some time to analyze which ones and what the value should be. So that would be a job for later...

@samsrabin
Copy link
Collaborator Author

@ekluzek Good point. I've now made new files, *c240207b.nc, with _FillValue only removed from variables that had it erroneously added in c240105. Does that work?

@samsrabin samsrabin merged commit c030c23 into ESCOMP:master Feb 8, 2024
2 checks passed
@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 bug something is working incorrectly PR status: awaiting review Work on this PR is paused while waiting for review.
Projects
Status: Done (non release/external)
Development

Successfully merging this pull request may close these issues.

Parameter file formatting has changed
3 participants