-
Notifications
You must be signed in to change notification settings - Fork 310
MTG LI L2: Hard coded the satellite position with the nominal vallues #3137
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3137 +/- ##
=======================================
Coverage 96.22% 96.22%
=======================================
Files 398 398
Lines 57383 57388 +5
=======================================
+ Hits 55218 55223 +5
Misses 2165 2165
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Pull Request Test Coverage Report for Build 15140857231Details
💛 - Coveralls |
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.
Still needs a unit test, and I suggested some other changes.
satpy/readers/li_base_nc.py
Outdated
logger.info("The satellite position is the nominal position and thus does not" \ | ||
"correspond to the actual position of the satellite. The exact satellite position can be found in " \ | ||
"FCI level 1 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.
I find the formulation here unclear. We should remember that logging messages may appear out of context for most users (they don't call add_orbital_parameters directly, but it gets called as part of larger processing). Maybe something like:
Assuming nominal satellite position for LI L2. The exact position is available from FCI L1.
And I would report this in debug mode, not info mode, as most users will not care about the difference. One use case is parallax correction — for this use case, the difference is too small to be relevant.
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.
Thank you for the suggestion. I will change this :)
data_array.attrs["orbital_parameters"] = {"satellite_nominal_longitude": 0.0, | ||
"satellite_nominal_latitude": 0.0, | ||
"satellite_nominal_altitude": 35786400.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.
If at least the satellite number is known from the metadata (is it?) this hardcoded position could be sorted by satellite. MTG-I2 / Meteosat-14 will have a different position.
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.
Good point, I didn't think about the fact that other satellite's products would require the use of the LI reader... But it is true that in any case, it would be better to have a more general solution because apparently the same problem holds for any L2 product of the MTG.
However, I am not sure to understand, how does the satellite number can be used to for this application? By writing a dict of satellites containing a dict of nominal position for each satellite?
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.
Yes, I would have a dict of satellites. Those should rarely change, although if they do, it will cause more of a headache as we would need to cover old and new 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.
yeah, when we start moving the MTGs between the nominal, rapid scan , IODC position etc, we'll have a new headache. But maybe by then we can include the information in the file format... So for now a simple dict mapping the platform_name
to the satellite_nominal_longitude
should be good for a while. Note that the platform_name
was added only recently in #2993
satpy/readers/li_base_nc.py
Outdated
@@ -733,6 +733,17 @@ def update_array_attributes(self, data_array, ds_info): | |||
|
|||
return data_array | |||
|
|||
def add_orbital_parameters(self, data_array): | |||
"""Hard code the nominal position of the satellite in the orbital parameters attribute.""" | |||
logger.info("The satellite position is the nominal position and thus does not" \ |
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.
logger.info("The satellite position is the nominal position and thus does not" \ | |
logger.info("The satellite position is the nominal position and thus does not " |
without a trailing space (or a leading one on the next line), it's going to output "does notcorrespond to".
satpy/readers/li_base_nc.py
Outdated
def add_orbital_parameters(self, data_array): | ||
"""Hard code the nominal position of the satellite in the orbital parameters attribute.""" | ||
logger.info("The satellite position is the nominal position and thus does not" \ | ||
"correspond to the actual position of the satellite. The exact satellite position can be found in " \ |
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.
"correspond to the actual position of the satellite. The exact satellite position can be found in " \ | |
"correspond to the actual position of the satellite. The exact satellite position can be found in " |
satpy/readers/li_base_nc.py
Outdated
@@ -766,5 +777,8 @@ def get_dataset(self, dataset_id, ds_info=None): | |||
# Update the attributes in the final array: | |||
data_array = self.update_array_attributes(data_array, ds_info) | |||
|
|||
#Add the nominal sat position in attrs orbital parameters |
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.
#Add the nominal sat position in attrs orbital parameters | |
# Add the nominal sat position in attrs orbital parameters |
In the MTG LI L2 reader
li_l2_nc
, I have hard coded the nominal satellite position as given in the L1 FCI dataset. It allows to use functions such asget_satpos
fromsatpy.utils
that is necessary to apply a parallax correction to the data.