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

GEOMS reader for TOLNet #57

Merged
merged 20 commits into from
Apr 5, 2022
Merged

GEOMS reader for TOLNet #57

merged 20 commits into from
Apr 5, 2022

Conversation

zmoon
Copy link
Member

@zmoon zmoon commented Mar 15, 2022

Closes #50

  • implement a open_dataset function to read a GEOMS file using optional dependency pyhdf
    • create and start using a helper fn for checking optional deps that prints helpful message if not installed
  • convert fakeDims to the real dims, giving a coordinate variable to each
  • convert times to datetime64
  • docs
  • account for LATITUDE/LONGITUDE without .INSTRUMENT (in Section 4.2.6 here we can see that the .INSTRUMENT may not always be there, or maybe one could even have both)

@zmoon
Copy link
Member Author

zmoon commented Mar 15, 2022

Was getting some CI failures because the AERONET website was temporarily unavailable. Maybe should find a way to skip those tests if that is the case (though linkcheck of the docs would still fail).

@zmoon
Copy link
Member Author

zmoon commented Mar 15, 2022

Python 3.6 is failing because it is reporting the dataset dims as ('altitude', 'time') instead of ('time', 'altitude').

Probably due to difference in xarray version somehow, since CPython 3.6 remembers dict insertion order too. Currently Getting:

  • 3.6
    • numpy 1.19.5
    • pandas 1.1.5
    • xarray 0.18.2
  • 3.8
    • numpy 1.22.3
    • pandas 1.4.1
    • xarray 2022.3.0

@zmoon
Copy link
Member Author

zmoon commented Mar 15, 2022

Here's what the test dataset is looking like when loaded:

image

@zmoon zmoon marked this pull request as ready for review March 15, 2022 21:33
@zmoon zmoon requested a review from bbakernoaa March 15, 2022 21:33
@bbakernoaa
Copy link
Member

I think that in MONET we may need to add logic to recognize or search for latitude_instrument for the interpolation routines as in the beginning of the monet_accessor.py https://github.com/noaa-oar-arl/monet/blob/stable/monet/monet_accessor.py#L76

@zmoon
Copy link
Member Author

zmoon commented Mar 29, 2022

I think that in MONET we may need to add logic to recognize or search for latitude_instrument for the interpolation routines as in the beginning of the monet_accessor.py

I think first we need to know whether any datasets could have data in both latitude and latitude_instrument, which does seem possible from the GEOMS description. I guess e.g. a lidar that is not pointed straight up?

@bbakernoaa
Copy link
Member

bbakernoaa commented Mar 29, 2022 via email

@bbakernoaa bbakernoaa assigned bbakernoaa and unassigned bbakernoaa Mar 29, 2022
@bbakernoaa
Copy link
Member

@jo3hnsullivan could you offer some advice here?

@jo3hnsullivan
Copy link

Well it could be a lidar on a moving platform so varying latitude and longitude. I'm not sure if they would have both latitude and latitude_instrument. Maybe one would point to the origin and the other would point to the time varying latitude and longitude

On Tue, Mar 29, 2022 at 2:19 PM Zachary Moon @.> wrote: I think that in MONET we may need to add logic to recognize or search for latitude_instrument for the interpolation routines as in the beginning of the monet_accessor.py I guess first we need to know whether any datasets could have data in both latitude and latitude_instrument, which does seem possible from the GEOMS description. I guess e.g. a lidar that is not pointed straight up? — Reply to this email directly, view it on GitHub <#57 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFIUVN75UNXS5OOH6THXUOTVCNCQ3ANCNFSM5QXIBROA . You are receiving this because your review was requested.Message ID: @.>

I have not come across any GEOMS files in the lidar community that use a separate latitude_instrument and latitude value.
There are lidars that could use this, such as a scanning doppler wind lidar, but in that case you are also adding a variable longitude, latitude, and time based on the scanning angle.

Is this documentation helpful? https://evdc.esa.int/documentation/browse-meta-data/data_group/

@bbakernoaa
Copy link
Member

@jo3hnsullivan do you want to add Guillaume here?

@jo3hnsullivan
Copy link

Any thoughts here @drguigui?

@drguigui
Copy link

I don't really know how NOAA is doing it: I think they prefer to put an angle from the vertical and maybe an azimuth: that's probably easier than putting some separate lat/lon.
I don't think it is a problem for ground lidar.
However, the question is with airplane lidars: they will have varying lat/lon. If we expect them to have GEOMS files, then they will have variations

@zmoon
Copy link
Member Author

zmoon commented Mar 29, 2022

I think that in MONET we may need to add logic to recognize or search for latitude_instrument for the interpolation routines

@bbakernoaa Maybe in the case of latitude_instrument/longitude_instrument only, another solution could be to just rename them in the reader to latitude/longitude.

@bbakernoaa
Copy link
Member

bbakernoaa commented Mar 29, 2022 via email

@zmoon
Copy link
Member Author

zmoon commented Mar 30, 2022

I have not come across any GEOMS files in the lidar community that use a separate latitude_instrument and latitude value.

That is good to know. I think we would be pretty safe just renaming latitude_instrument to the MONETIO-preferred latitude then (checking first that it isn't already there).

the question is with airplane lidars: they will have varying lat/lon. If we expect them to have GEOMS files, then they will have variations

I think they prefer to put an angle from the vertical and maybe an azimuth: that's probably easier than putting some separate lat/lon.

I will have to make modifications to support other possible dimensions that GEOMS supports. With my current approach, variations in lat/lon should be possible to handle, but it would be good to have a file to test.

@bbakernoaa
Copy link
Member

@zmoon I say we go ahead and proceed. If/when we come across a Lidar at an angle we can then spend the time to update it. I think that this should be ready to go once we rename the latitude_instrument and longitude_instrument

in the case that we don't already have a latitude dim
@zmoon
Copy link
Member Author

zmoon commented Mar 30, 2022

ready to go once we rename the latitude_instrument and longitude_instrument

Ok, that is done. I still worry that current code won't work properly for datasets with different dimensions, but I guess we can wait until we are provided such a file to test to address that.

@zmoon zmoon changed the title GEOMS reader GEOMS reader for TOLNet Mar 31, 2022
@zmoon
Copy link
Member Author

zmoon commented Apr 5, 2022

Ok, this should at least work for TOLNet files similar to our sample file, so I am going to merge this now. We can enhance it to support other files when they come up.

@zmoon zmoon merged commit 9f37302 into noaa-oar-arl:develop Apr 5, 2022
@zmoon zmoon deleted the geoms branch April 5, 2022 21:34
@zmoon zmoon mentioned this pull request Oct 4, 2023
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.

None yet

4 participants