pysat / pysatCDAAC

pysat support for CDAAC instruments
BSD 3-Clause "New" or "Revised" License
2 stars 2 forks source link

ENH: add support for cosmic2 ivm #32

Closed jklenzing closed 1 year ago

jklenzing commented 2 years ago

Description

Addresses #31. Downstream of #30. Will rebase into develop once that is merged.

Putting up for discussion. Known issues:

Type of change

How Has This Been Tested?

via pytest. Requires pysat/pysat#950

Test Configuration:

Checklist:

rstoneback commented 2 years ago

You could write a download method within cosmic2_ivm that invokes the general method, and then afterwards, goes through and makes sub-directories and moves files.

jklenzing commented 2 years ago

You could write a download method within cosmic2_ivm that invokes the general method, and then afterwards, goes through and makes sub-directories and moves files.

I started writing this earlier, but wound up stuck on how to generate the destinations for each instrument without instantiating new instruments for each.

rstoneback commented 2 years ago

You could write a download method within cosmic2_ivm that invokes the general method, and then afterwards, goes through and makes sub-directories and moves files.

I started writing this earlier, but wound up stuck on how to generate the destinations for each instrument without instantiating new instruments for each.

You could search for inst_id value in the passed in data_path, swap out that area for the other instruments.... There are probably issues with the approach... I'll give it some thought on my end. 🐵

jklenzing commented 2 years ago

@rstoneback, I can convert from GPS time ('time') or UNIX time ('uts') for the index. Any preference?

rstoneback commented 2 years ago

Not on my end.

On Oct 27, 2021, at 1:46 PM, Jeff Klenzing @.***> wrote:

@rstoneback https://github.com/rstoneback, I can convert from GPS time ('time') or UNIX time ('uts') for the index. Any preference?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pysat/pysatCDAAC/pull/32#issuecomment-953212179, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3N26LYRFWSWPJC4MT2WVTUJBCGRANCNFSM5GYT3FRQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

jklenzing commented 2 years ago

I pushed the option that works (the other gives me a non-unique index). We'll need to sort out pysat/pysat#944 for the long-haul, but I suspect this branch will hang around for a while as we update standards. Gives us somethign to load and compare to in the meantime.

jklenzing commented 2 years ago

Noting that docs don't exist yet for this package. Adding to the issues.

jklenzing commented 2 years ago

Noting that this still needs appropriate acknowledgements and references

jklenzing commented 2 years ago

Acknowledgements based on the first results paper here: https://agupubs.onlinelibrary.wiley.com/doi/full/10.1029/2019GL086841

For the reference, I am using the persistent DOI for the dataset which redirects here: https://www.cosmic.ucar.edu/what-we-do/cosmic-2/data The citation is based on their suggested format at the bottom of the page.

The first results paper does not include IVM results, so it's not here. We should include it when we add the GPS data in the next pull.

Thoughts? Is there anything I've missed? @aburrell, @rstoneback

jklenzing commented 2 years ago

TODO: add inst_id separation

rstoneback commented 2 years ago

I built a COSMIC-2 constellation for a seasonal analysis demo for the paper. Worked well. Data averages popped out no problem. 👍

jklenzing commented 1 year ago

TODO:

jklenzing commented 1 year ago

test branch at https://github.com/pysat/pysatCDAAC/tree/nomerge/sort_files

jklenzing commented 1 year ago

After reviewing previous test branches and playing with syntax, here are the options (assuming we don't want more breaking changes in pysat).

Options for inst_id sorting:

I am in favor of either of the first two, as those can be implemented in a straightforward fashion. The other two push us away from reusable code, and will make development / maintenance / usage harder. I am not convinced that maintaining an inst_id subdirectory buys us anything. The daily subdirectory creates a better consistency within the package.

aburrell commented 1 year ago

Move files after download to maintain inst_id grouping. This requires custom hard-wired code duplicating information in the instrument module, plus other instruments need to be refreshed or users will download again.

I am not sure I understand this. Maybe we could arrange a call at some point or put this on the agenda?

rstoneback commented 1 year ago

I am not convinced that maintaining an inst_id subdirectory buys us anything. The daily subdirectory creates a better consistency within the package.

If we don't maintain the directory format as specified by the user then cosmic-2 ivm will have an inconsistent structure. I see 'directory_format : {platform}/{name}/{tag}/{inst_id}' in my pysat.params.

The cosmic_gps instrument support doesn't include support for inst_ids.

Even without moving the directory structure all of cosmic2 IVMs would need a files refresh after a download.

Happy to talk more.

jklenzing commented 1 year ago

This branch is ready for review

jklenzing commented 1 year ago

Reverting back to storing files in a single directory. Maintaining an inst_id directory for this instrument involves adding multiple functions specifically for this instrument, creating maintenance problems. pysat supports alternate structures for instruments, which we are using elsewhere. I think accepting this structure here will make the code easier to maintain.