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

Computation of Earth scene radiance #62

Closed
helgaweb opened this issue May 20, 2020 · 9 comments · Fixed by #58
Closed

Computation of Earth scene radiance #62

helgaweb opened this issue May 20, 2020 · 9 comments · Fixed by #58
Assignees
Labels

Comments

@helgaweb
Copy link

I was wondering, why the Earth scene radiance (Ne) does not account for linear radiance estimate Nlin as described in the KLM User's Guide. Following 7.1.2.4-6 on page 7-9, Ne is calculated as: Ne = Nlin + Ncor

pygac/pygac/calibration.py

Lines 553 to 558 in 2d7fbaf

Nlin = (cal.n_s[chan] +
(((nBB - cal.n_s[chan])
* (new_space - counts.astype(float)))
/ (new_space - new_ict)))
Ncor = cal.b0[chan] + Nlin * (cal.b1[chan] + cal.b2[chan] * Nlin)
Ne = Ncor

@carloshorn
Copy link
Collaborator

Hi @helgaweb, indeed, to me it also looks like Nlin is missing. Currently, I am working on a refactoring of calibration.py, where I want to put more comments on the equations. Hope to push the corresponding commit to #58 today.

@helgaweb
Copy link
Author

@carloshorn that will be very helpful, thanks! I think the documentation in the KLM User's guide is quite well done and provides a good basis.
Alright, let's wait for @abhaydd and @mraspaud ideas on this and then I could open a PR (after you finished the refactoring).

@carloshorn
Copy link
Collaborator

I checked the code, and it is doing the right thing, but written in a confusing way...
The coefficients are defined here:

pygac/pygac/calibration.py

Lines 304 to 306 in 2d7fbaf

'b0': np.array([0.0, 4.76, 3.83]),
'b1': np.array([1 - 0.0, 1 - 0.0932, 1 - 0.0659]),
'b2': np.array([0.0, 0.0004524, 0.0002811]),

Which shows, that the linear correction 'b1' term is added by one, which therefore accounts for the linear part. However, I agree that there should be a huge alert sign in the code.

@abhaydd
Copy link
Collaborator

abhaydd commented May 22, 2020

@helgaweb @carloshorn Yes, the Nlin is of course included properly (but implemented in an unclear way). Otherwise we would have been off by hundreds of degrees. The non-linearity correction is typically only a fraction and translates to less than few Kelvins.

@helgaweb
Copy link
Author

@abhaydd Thanks a lot. Sorry I've overseen the change in the b1 coefficient (as clarified by @carloshorn) and was therefore confused about the calculation of Ne.

@carloshorn
Copy link
Collaborator

@helgaweb, I pushed the comment lines to #58. I would appreciate if you could have a look at it and leave a feedback telling if the comments are sufficient to understand what the functions do.

@abhaydd
Copy link
Collaborator

abhaydd commented May 22, 2020

@helgaweb @carloshorn I can understand why one can get confused. The code clearly needs revamping. And it’s good to have the fresh minds like yours and Carlos’s :)

@helgaweb
Copy link
Author

@carloshorn Sure. I hope to give you some feedback by monday!

@mraspaud
Copy link
Member

I'm reopening this as I think it's a problem that it isn't documented clearly that the coeffs are taking acount of this. Hopefully #58 closes this.

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 a pull request may close this issue.

4 participants