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

Add tm5_constant_a and tm5_constant_b for tropomi_l2 #1128

Merged
merged 1 commit into from
Apr 3, 2020

Conversation

zxdawn
Copy link
Member

@zxdawn zxdawn commented Apr 2, 2020

Because of different shape, tm5_constant_a and tm5_constant_b are ignored by the tropomi_l2.py reader.
I added them in tropomi_l2.yaml now.

@zxdawn zxdawn requested review from djhoese and mraspaud as code owners April 2, 2020 11:56
@djhoese
Copy link
Member

djhoese commented Apr 2, 2020

CC @tommyjasmin for opinions.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.003%) to 89.532% when pulling e83e49c on zxdawn:tropomi into d12bb7a on pytroll:master.

@mraspaud mraspaud added component:readers enhancement code enhancements, features, improvements labels Apr 2, 2020
@mraspaud mraspaud added this to the v0.21.0 milestone Apr 2, 2020
@codecov
Copy link

codecov bot commented Apr 2, 2020

Codecov Report

Merging #1128 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1128   +/-   ##
=======================================
  Coverage   89.53%   89.53%           
=======================================
  Files         200      200           
  Lines       29356    29356           
=======================================
  Hits        26284    26284           
  Misses       3072     3072

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d12bb7a...e83e49c. Read the comment docs.

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

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

LGTM

@ghost
Copy link

ghost commented Apr 3, 2020

Congratulations 🎉. DeepCode analyzed your code in 0.007 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard

@mraspaud mraspaud merged commit e804f03 into pytroll:master Apr 3, 2020
@tommyjasmin
Copy link
Contributor

Hi all - @zxdawn Xin - can you explain the use of these constants and which products they are expected to be present in? In the current YAML, we reference them directly under the PRODUCT group:

'PRODUCT/tm5_constant_a'

This appears to be correct for NO2 data, which I presume you are working with.

But with a very brief look, I do not see these constants in the CO sample data I have. I do see these constants in my SO sample data. However, for those files they are present in a support data subgroup:

'PRODUCT/SUPPORT_DATA/INPUT_DATA/tm5_constant_a'

So we can't assume these constants are always present in the top level PRODUCT group.

@zxdawn zxdawn deleted the tropomi branch April 4, 2020 02:48
@zxdawn
Copy link
Member Author

zxdawn commented Apr 4, 2020

@tommyjasmin Hi Tommy, Yes, I tested it for NO2 files.

The constants are used to calculate the TM5 model pressure levels which are used in the NO2 retrieval, like this:

P = A + B*PSFC

I will fix this and test all kinds of tropomi products soon. Thanks.

@zxdawn
Copy link
Member Author

zxdawn commented Apr 4, 2020

@tommyjasmin I've tested the newest YAML and it works well for NO2, SO2, O3 and CO data. Because the var_name in YAML is tested whether exists in the file:

if (matches and var_name in self) or (assembled and bounds_exist):

So, is it OK to just keep these two variables in the YAML? @mraspaud @djhoese

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:readers enhancement code enhancements, features, improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants