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

Set lev positive 'up' for all collections except Emissions #10

Merged
merged 1 commit into from Jul 16, 2021

Conversation

lizziel
Copy link

@lizziel lizziel commented Jul 16, 2021

This PR is a quick fix to issue geoschem/GCHP#112. With this update the lev dimension positive attribute in History (diagnostic) files is set to up for all collections except Emissions which remains down. Previously all collection output files were set to down by default which is correct for GEOS but not GCHP.

Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
@lizziel
Copy link
Author

lizziel commented Jul 16, 2021

Please note that this PR is a quick fix for GCHP so that the files are correct. A more elegant solution from GMAO will likely replace it in the future. See GEOS-ESM#914.

Copy link

@LiamBindle LiamBindle left a comment

Choose a reason for hiding this comment

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

@lizziel I think this is a good temporary fix. The fact that 3d emission diagnostics are upside-down relative to other collections is definitely confusing and this make it more clear.

We should note here that positive refers to the index direction not the direction of increasing lev values. Technically that's not CF-compliant but it's better than having it always down. I don't see an easy way to reverse the lev coordinate if posDown=.FALSE.. If you know how I think it would be worthwhile for the CF-compliance but I don't think it necessary.

As you said GEOS-ESM#914 seems like the proper long-term solution but I don't see any reason not to include this in 13.2.

edit: Corrections. See below.

@lizziel
Copy link
Author

lizziel commented Jul 16, 2021

We should note here that positive refers to the index direction not the direction of increasing lev values. Technically that's not CF-compliant but it's better than having it always down. I don't see an easy way to reverse the lev coordinate if posDown=.FALSE.. If you know how I think it would be worthwhile for the CF-compliance but I don't think it necessary.

I'm not quite following. The lev array is 1,2,...,72 whether the positive attribute is up or down. SpeciesConc(:,:,1) is values for lev(1) which happens to be level 1 for our files. Positive up now makes that defined as surface. Positive down incorrectly defined that at TOA. Doesn't that make the positive direction correspond to vertical direction in which the lev coordinate values are increasing (CF-compliant definition) at least in the scope of the file? Maybe I'm missing something?

@lizziel lizziel merged commit 0058605 into gchp/dev Jul 16, 2021
@LiamBindle
Copy link

Oops, my mistake. Thanks for the correction. I was confused and I thought the order of the coordinate also needed to be reversed.

Drawing it out helped me understand the clarification:

           (BOA)        (TOA)
positive:    1,   ...  , 72
down:       72,   ...  , 1           (equivalent)

           (TOA)        (BOA)
down:        1,   ...  , 72
positive:   72,   ...  , 1           (equivalent)

This PR is CF-compliant then. In that case, I think it's a good solution since semantically it's correct. I updated my previous comment.

Refs:

@lizziel
Copy link
Author

lizziel commented Jul 16, 2021

I think using lev values as index is inherently confusing. Pressures are much easier to understand.

@lizziel
Copy link
Author

lizziel commented Jul 16, 2021

I want to add to these comments another couple reasons the fix in this PR should be temporary.

  1. If you add HEMCO emissions collections with names other than "Emissions" then positive will be "up" which is incorrect.
  2. If you had collections for non-GEOS-Chem grid comps, e.g. FV, then positive will be "up" which is incorrect.

For these cases you need to add the new collection name to the MAPL condition for keeping positive "down". Not good!

@LiamBindle LiamBindle deleted the bugfix/history_lev_pos_up_except_emissions branch March 21, 2022 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants