-
Notifications
You must be signed in to change notification settings - Fork 26
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
Fix selection of user defined scanlines #12
Conversation
In the presence of scanlines with invalid lat/lon info, user defined scanlines are not selected correctly. - Fix selection of user defined scanlines - Add tests covering that problem
@@ -77,10 +89,17 @@ def save_gac(satellite_name, | |||
mask, qual_flags, start_line, end_line, tsmcorr, | |||
gac_file, midnight_scanline, miss_lines, switch=None): | |||
|
|||
|
|||
along_track = lats.shape[0] | |||
last_scan_line_number = qual_flags[-1, 0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I remember, the last_scan_line_number attribute was included to enable the user to reconstruct the initial scanline number range. But if it is determined after slicing [temp_start_line:temp_end_line], scanline numbers > temp_end_line are not taken into account. That's why I moved the line to the top of the method. If you think, it should just have the value of the last scanline number in the hdf5 file, we can move it to the end of the method.
@@ -112,21 +131,19 @@ def save_gac(satellite_name, | |||
ref3[switch == 2] = MISSING_DATA | |||
bt3[switch == 2] = MISSING_DATA | |||
|
|||
for array in [ref1, ref2, ref3, bt3, bt4, bt5]: | |||
array[np.isnan(array)] = MISSING_DATA |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved all statements masking invalid data to this block
pygac/gac_io.py
Outdated
qual_flags[0:temp_start_line, 0].tolist() + | ||
miss_lines.tolist() + | ||
qual_flags[temp_end_line+1:, 0].tolist() | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Include scanlines with invalid lat/lon info into the list of missing scanlines
start_line=start_line, | ||
end_line=end_line, | ||
temp_start_line=temp_start_line, | ||
temp_end_line=temp_end_line) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the actual fix.
For some reason the tsm correction tests sometimes fail on the first attempt... Had to close & reopen to trigger a re-run. |
Set to None if it has been removed due to invalid lat/lon info
Sorry for taking so long to look at this. I merged the other PRs, but this one seems to be conflicting somehow... |
No problem! I'll have a look at it next week. Have to remember what I was doing here 😄 |
Resolved the merge conflicts and fixed some minor issues with the midnight scanline computation we discovered recently. Some calibration tests fail, I guess because you updated the coefficients. Unfortunately I have deleted the original branch I made the PR from. Can you assign a new branch to this PR or shall I create a new PR? The branch is: https://github.com/sfinkens/pygac/tree/fix-scanline-selection |
It doesn't seem like we can change the source of the PR. If you can't either (see top of the PR, click "Edit" maybe?) then you may have to create a new one. |
Thanks @djhoese. I can only change the pygac branch I want to merge into. So I will open a new PR. |
I believe there is a bug in
pygac.gac_io.save_gac
. User defined scanlines are not selected correctly in the presence of scanlines with invalid lat/lon info:data = data[temp_start_line:temp_end_line]
start_line/end_line
to new slicedata = data[start_line, end_line]
I believe step 2 is not done correctly. Here is a sketch with the details: pygac_2step_slice.pdf
The consequence for CLARA-A2 is, that we get scanlines beyond the requested range from pygac. As the range was chosen to avoid overlap between consecutive orbits, we re-introduce an overlap of
temp_start_line
scanlines. I did a quick CLARA-A2 logfile analysis:In 2011 almost every orbit from every platform is affected, but luckily the average temp_start_line is 15, which is not a serious problem. In 1984, the additional overlap is quite significant, but only very few orbits are affected.
Cloud_cci is not affected at all, because they always process all scanlines.