NOAA-ORR-ERD / LibGOODS

Library for accessing data useful for the NOAA / GNOME model
https://libgoods.readthedocs.io/en/latest/
Other
1 stars 2 forks source link

Rotates longitude to be [-180, 180] #46

Closed lukecampbell closed 1 year ago

lukecampbell commented 1 year ago

This commit addresses 2 concerns:

ChrisBarker-NOAA commented 1 year ago

Thanks!

What if you pass in (180, -10, 320, 20) to a model that's using -180->180 ?

And:

output files written to disk that have longitude values [0, 360] will be written as [-180, 180] if and only if the coordinate values remain monotonically increasing/decreasing.

What does happen? you get a file without the coords transformed? If so, a warning?

lukecampbell commented 1 year ago

For a model using [-180, 180], If you pass (180, -10, 320, 20) the subset will use it as is, and it will return an empty set and probably raise an error.

An warning is printed, and the output file retains longitude coordinate values in the domain of [0, 360]

ChrisBarker-NOAA commented 1 year ago

OK -- we can leave being smarter for a future feature.

But maybe an error rather than a warning? Or maybe an empty file is error enough?

I'm thinking, e.g. (160, -10, 200, 20) -- so you would get something, but not what you expected.

perhaps a warning is fine there, though.

lukecampbell commented 1 year ago

If the model uses [0, 360] and you pass a bbox selection of (160, y1, 200, y2), the resulting output after subsetting would be a warning message that the longitude values couldn't be translated because of their not monotonic and the file would contain longitude values [160, 200].

If the model uses [-180, 180] and you pass a bbox selection of (160, y1, 200, y2), the resulting output after subsetting would be a smaller than maybe expected result containg longitude values [160, 180].

If the bbox is completely disjoint from the model's domain, it returns a valid netCDF file with horizontal dimensions of size 0. I don't think that's desirable. Before writing to disk, I think I'd like to add a simple check that there's at least some real data left before writing the file. For one it will save the user's time, xarray still takes like 3 seconds to go fetch all the scalars and such to write to disk even though the file is arguably useless.

ChrisBarker-NOAA commented 1 year ago

If the bbox is completely disjoint from the model's domain, it returns a valid netCDF file with horizontal dimensions of size 0. I don't think that's desirable. Before writing to disk, I think I'd like to add a simple check that there's at least some real data left before writing the file.

+1 -- raising an error would be much better than returning an empty file.

If the model uses [-180, 180] and you pass a bbox selection of (160, y1, 200, y2), the resulting output after subsetting would be a smaller than maybe expected result containing longitude values [160, 180].

Here's where I think we could be a little smarter, though that may need to wait another day:

So:

1) Translate the user's supplied BB into the coords the model uses 2) Do the subset 3) Translate the results back into the coords the user requested.

When in doubt, default to -180--180

Raise an error if the result is empty. (we want any anyway just to catch mistakes)

Raise an error (I think,maybe warning???) if the user asks for what would be a disjoin result, e.g.

The question is whether to raise an error or return a disjoint result, so the user can fix it themselves later.

lukecampbell commented 1 year ago

I agree, I think though it will need to supported in a separate feature. Those cases require a more careful and sophisticated approach than what I have in this PR.

I found a bug in the PR with FVCOM models, once I patch it, this PR should be good to go.

ChrisBarker-NOAA commented 1 year ago

sound good -- I captured the potential future step in #48