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

TROPOMI updates including pressure calculation #184

Merged
merged 19 commits into from
Sep 19, 2024
Merged

Conversation

zmoon
Copy link
Member

@zmoon zmoon commented Aug 12, 2024

monetio/models/_wrfchem_mm.py Outdated Show resolved Hide resolved
monetio/models/_wrfchem_mm.py Outdated Show resolved Hide resolved
@zmoon
Copy link
Member Author

zmoon commented Aug 27, 2024

Thanks @blychs for the comments.

@zmoon zmoon marked this pull request as ready for review August 27, 2024 18:07
@zmoon zmoon mentioned this pull request Aug 28, 2024
@blychs
Copy link
Contributor

blychs commented Sep 5, 2024

Hi @zmoon , I was wondering on how exactly are you planning on adding PH and PHB to the _wrfchem_mm reader. I need equivalent variables for the _camx_mm reader, and was just calling them "layer_height_agl" and "topo_height" as part of solving Issue #189 , but would rather make the names consistent with whatever you make. I also need to choose on those names for the TEMPO MM utility. (That is a bit different from just adding PH and PHB, since PH and PHB include the topographic height as bottom_top_stagg=0)
We would need to update every reader if we want TROPOMI and TEMPO to work with all of them, and having consistency would be useful.
It shouldn't be too hard to make everything consistent to whatever choice you make.

@zmoon
Copy link
Member Author

zmoon commented Sep 9, 2024

how exactly are you planning on adding PH and PHB to the _wrfchem_mm reader

@blychs I was just following what Meng had done, which was just add them to the var list. So I was assuming that means they are included in the model output or can be automatically derived via wrf-python getvar().

I need equivalent variables for the _camx_mm reader, and was just calling them "layer_height_agl" and "topo_height"

The reader already has height AGL and above MSL. I don't think the height of the topography comes out though, at least by default.

If @mlirenzhenmayi was only using PH, PHB to compute height as (PH+PHB)/g0, maybe we don't actually need them? And similar for P, PB.

@blychs
Copy link
Contributor

blychs commented Sep 9, 2024

@zmoon I thought that was mid layer height, whereas PH and PHB are the geopotential at the layer interface. This is quite useful, because then (PH + PHB)[:, 1:, :, :] - (PH + PHB)[:, -1, :, :]) / g0 is the layer thickness, which is needed for calculating the partial column. If this is not the case, please let me know, because my _camx_mm reader assumes it is, and I would have to change that (that is, I calculated the height at mid layer for that variable).
(PH + PHB) / 9.81 at bottom_top_stagg=0 should be pretty much the topography, if I'm not wrong.

I don't think we need P, PB though. P + PB is just the pressure.

@zmoon
Copy link
Member Author

zmoon commented Sep 9, 2024

@blychs I believe you are correct about the calculation. But if it's really the layer height thickness is needed, maybe we should have the reader return that instead, unless there is a chance of using the layer interface heights for something else.

Also for WRF-Chem reader it looks like getvar will give you the layer interface heights if you request "zstag". So maybe here we should use that.

@blychs
Copy link
Contributor

blychs commented Sep 9, 2024

@zmoon I am not sure about the calculations. Do you think that having the layer height could be useful to interpolate to other (intermediate) altitudes? I'm thinking mostly about the aircraft team - I know close to nothing about aircraft field campaigns. If it isn't, then changing this to layer thickness wouldn't be to hard.

@mlirenzhenmayi
Copy link

@zmoon @blychs PH and PHB are used to calculate the height for each layer, and then we can calculate the partial column for each layer. Pressure for each layer = PB + P, from WRF-Chem. It's used for averaging kernels revision, based on which we established a function of Pressure and averaging kernel for each layer from TROPOMI, interpolate the model pressure to TROPOMI pressure for each layer, and then get the layered averaging kernel for the model.

@zmoon
Copy link
Member Author

zmoon commented Sep 10, 2024

Hi @mlirenzhenmayi thanks for the comments. Is it correct that PB + P is the pressure of the layer interfaces? To use for conservative vertical interp based on pressure, for example.

@rschwant and I think these calculations of interface height/pressure and layer height/pressure thickness might be better done inside the reader and assigned standardized variable names, like currently done for the mid-layer pressure/height. But that's outside the scope of this PR.

@mlirenzhenmayi
Copy link

Hi @mlirenzhenmayi thanks for the comments. Is it correct that PB + P is the pressure of the layer interfaces? To use for conservative vertical interp based on pressure, for example.

@rschwant and I think these calculations of interface height/pressure and layer height/pressure thickness might be better done inside the reader and assigned standardized variable names, like currently done for the mid-layer pressure/height. But that's outside the scope of this PR.

PB+P is the mid-layer pressure. Maybe it's the tricky part of WRF-Chem. WRF-Chem doesn't provide the standard mid-layer pressure, so we need to calculate it based on the PB and P. If there's a standard calculation for mid-layer pressure covering different models, we can use that instead of calculating separately.

@rschwant
Copy link
Contributor

@mlirenzhenmayi I was more thinking that we can move your calculation for these variables into the WRF-Chem reader, since they are model dependent. And then each model will do these calculations differently. And the analysis code will just use the standard value across all the models. Does that make sense? This can be a version 2 satellite advancement once we start standardizing the code more too.

@mlirenzhenmayi
Copy link

@mlirenzhenmayi I was more thinking that we can move your calculation for these variables into the WRF-Chem reader, since they are model dependent. And then each model will do these calculations differently. And the analysis code will just use the standard value across all the models. Does that make sense? This can be a version 2 satellite advancement once we start standardizing the code more too.

I think that makes sense. I do have a separate calculation module for this: https://github.com/NOAA-CSL/MELODIES-MONET/blob/develop/melodies_monet/util/cal_mod_no2col.py . It can be straight forward to combine cal_mod_no2col.py with the reader.

@zmoon
Copy link
Member Author

zmoon commented Sep 10, 2024

@mlirenzhenmayi I think the partial column calculation can be outside the reader (or inside, open to others' thoughts about that).

However, I do think it makes sense for the reader to provide the variables you need, instead of you needing to compute them from WRF-specific P,PB,PH,PHB,T.

  • temperature is already provided1 as temperature_k (computed via T by wrf-python getvar)
  • pressure is already provided as pres_pa_mid (computed via P+PB by wrf-python getvar)
  • layer $\Delta z$ is not yet provided, we can add this (taking the diff of wrf-python getvar 'zstag')

Footnotes

  1. When surf_only_nc is false.

@blychs blychs mentioned this pull request Sep 11, 2024
@mlirenzhenmayi
Copy link

@mlirenzhenmayi I think the partial column calculation can be outside the reader (or inside, open to others' thoughts about that).

However, I do think it makes sense for the reader to provide the variables you need, instead of you needing to compute them from WRF-specific P,PB,PH,PHB,T.

  • temperature is already provided1 as temperature_k (computed via T by wrf-python getvar)
  • pressure is already provided as pres_pa_mid (computed via P+PB by wrf-python getvar)
  • layer
    Δ
    z
    is not yet provided, we can add this (taking the diff of wrf-python getvar 'zstag')

Footnotes

  1. When surf_only_nc is false.

@zmoon The partial column calculation has already been calculated outside of the reader. It's done in cal_mod_no2col.py: https://github.com/NOAA-CSL/MELODIES-MONET/blob/develop/melodies_monet/util/cal_mod_no2col.py . I agree that the P, PB, PHB and PH should be read by default in the reader.

Copy link

@mlirenzhenmayi mlirenzhenmayi left a comment

Choose a reason for hiding this comment

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

I tested the changes with my previous codes, and the results are right. I only have two comments for code revision.

monetio/sat/_tropomi_l2_no2_mm.py Outdated Show resolved Hide resolved
for varname in ds.attrs["var_applied"]:
logging.debug(f"applying quality flag to {varname}")
values = ds[varname].values
values[quality_flag <= quality_thresh_min] = np.nan


def open_dataset(fnames, variable_dict, debug=False):

Choose a reason for hiding this comment

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

can we change the function name to read_trpdataset? Just to be consistent with driver which has been incorporated into melodies-monet

Copy link
Member Author

Choose a reason for hiding this comment

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

I added it as an alias: 626b95c

In general, we're trying to keep API's for the different model/sat readers somewhat consistent, with the same function names etc.

Copy link

@mlirenzhenmayi mlirenzhenmayi left a comment

Choose a reason for hiding this comment

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

Add one comment on pressure dimension

monetio/sat/_tropomi_l2_no2_mm.py Show resolved Hide resolved
Meng's original func name
@zmoon zmoon merged commit db64a0e into noaa-oar-arl:develop Sep 19, 2024
7 checks passed
@zmoon zmoon deleted the meng branch September 19, 2024 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants