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

CLI: get data from ISH and ISH-Lite #236

Merged
merged 12 commits into from
Jan 31, 2024
Merged

CLI: get data from ISH and ISH-Lite #236

merged 12 commits into from
Jan 31, 2024

Conversation

zmoon
Copy link
Collaborator

@zmoon zmoon commented Jan 17, 2024

melodies-monet get-ish and get-ish-lite

cc: @jordanschnell

xref: noaa-oar-arl/monetio#157

Closes #232

needed for MM time series, e.g.
The only difference between mine and the file Jordan shared
with me was for trying to get a box option work
it can time out in read though, even for this small subset
@zmoon
Copy link
Collaborator Author

zmoon commented Jan 17, 2024

@jordanschnell I compared the file you gave me to this branch and the only differences were related to being able to specify the bounding box. I was able to get a working implementation of the box argument and I added it to get-ish as well. Here's an example with a small box (which I also use in the test I added):

melodies-monet get-ish-lite -s 2023-01-01 -e '2023-01-01 23:00' --box 39.5 -105.75 40.5 -104.75 --debug --verbose

@zmoon
Copy link
Collaborator Author

zmoon commented Jan 18, 2024

@jordanschnell this non-Lite case with your box took 10 minutes with 12 procs on GMU Hopper.

melodies-monet get-ish -s 2024-01-01 -e '2024-01-01 23:00' --box 0 -140 90 -50 -n 12 --no-compress --verbose --debug

However, dates in past (full) years seem to take very long still (2019-08-07 case, mentioned in one of the files you shared, finished successfully but in almost 3 hours), though at least with noaa-oar-arl/monetio#157 it seems not to fail part of the way through. Should be able to get it faster after addressing the points in noaa-oar-arl/monetio#113

@zmoon
Copy link
Collaborator Author

zmoon commented Jan 18, 2024

Note: In the merge (aa33d33), undid the change in 9de4ea4 since that test should be working now.

Copy link
Collaborator

@jordanschnell jordanschnell left a comment

Choose a reason for hiding this comment

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

Works as expected, approved.

Copy link
Collaborator

@rschwant rschwant left a comment

Choose a reason for hiding this comment

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

Thanks Zach, this looks great. Thanks for adding this in!

@zmoon
Copy link
Collaborator Author

zmoon commented Jan 31, 2024

Thanks @jordanschnell and @rschwant

@zmoon zmoon merged commit f77fa3e into NOAA-CSL:develop Jan 31, 2024
4 checks passed
@zmoon zmoon deleted the cli-ish branch January 31, 2024 18:22
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.

3 participants