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

Removed functionality from MPAS framework that overwrites attributes #4987

Merged
merged 1 commit into from
Jun 28, 2022

Conversation

akturner
Copy link
Contributor

Removed functionality from MPAS framework that overwrites Registry attributes with stream file attributes. This change prevents input files from overwriting CF-compliant units in Registry with non-CF compliant ones in the input file.

Fixes #4986

[BFB]

Removed functionality from MPAS framework that overwrites Registry attributes with stream file attributes. This change prevents input files from overwriting CF-compliant units in Registry with non-CF compliant ones in the input file.
@xylar
Copy link
Contributor

xylar commented Jun 6, 2022

@akturner, thank s a lot for making this PR. I'd like to test it with some other CF changes from the Hackathon, which I'm still working on. It would be good to test with @darincomeau's units changes, too.

@rljacob
Copy link
Member

rljacob commented Jun 16, 2022

@xylar how are those tests going?

@xylar
Copy link
Contributor

xylar commented Jun 16, 2022

@rljacob, it's not. I have too much else on my plate at the moment.

Copy link
Contributor

@xylar xylar left a comment

Choose a reason for hiding this comment

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

@akturner, sorry for the delay. I tested this with standalone MPAS-Ocean in compass (in a test E3SM branch where I also merged #5026 and #5027), and it worked like a charm! I used an initial condition from before, where the units for salinity are:

	double salinity(Time, nCells, nVertLevels) ;
		salinity:long_name = "salinity" ;
		salinity:units = "grams salt per kilogram seawater" ;

The resulting output is:

	double salinity(Time, nCells, nVertLevels) ;
		salinity:long_name = "salinity grams salt per kilogram seawater"
 ;
		salinity:units = "1.e-3" ;

as desired.

In time series stats daily, I also see:

	double timeDaily_avg_activeTracers_salinity(Time, nCells, nVertLevels) ;
		timeDaily_avg_activeTracers_salinity:long_name = "salinity grams salt per kilogram seawater" ;
		timeDaily_avg_activeTracers_salinity:units = "1.e-3" ;

@xylar
Copy link
Contributor

xylar commented Jun 21, 2022

@mark-petersen and @matthewhoffman, please give this a try as well when you can.

Copy link
Contributor

@mark-petersen mark-petersen left a comment

Choose a reason for hiding this comment

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

I agree, this works as expected based on @xylar's test above. Thanks.

Copy link
Contributor

@matthewhoffman matthewhoffman left a comment

Choose a reason for hiding this comment

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

I'm approving by inspection and the testing that Xylar and Mark did. This is a valuable change.

@jonbob jonbob added bug fix PR BFB PR leaves answers BFB labels Jun 27, 2022
jonbob added a commit that referenced this pull request Jun 27, 2022
Remove functionality from MPAS framework that overwrites attributes

Removes functionality from MPAS framework that overwrites Registry
attributes with stream file attributes. This change prevents input files
from overwriting CF-compliant units in Registry with non-CF compliant
ones in the input file.

Fixes #4986

[BFB]
@jonbob
Copy link
Contributor

jonbob commented Jun 27, 2022

passes sanity testing, merged to next

@jonbob jonbob merged commit 87dad35 into E3SM-Project:master Jun 28, 2022
@jonbob
Copy link
Contributor

jonbob commented Jun 28, 2022

merged to master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MPAS input overwrites registry attributes
6 participants