search

MITgcm / xmitgcm

Read MITgcm mds binary files into xarray
http://xmitgcm.readthedocs.io
MIT License
56 stars 65 forks source link

added fix for reading mds data with a large number of vertical levels. #310

Closed fraserwg closed 2 years ago

fraserwg commented 2 years ago

This resolves issue #309.

timothyas commented 2 years ago

Hey @fraserwg, sorry this has been sitting around with no attention for a while. I'm curious, what happens when you read in a dataset in this case? Does xmitgcm properly figure out how many levels there are?

fraserwg commented 2 years ago

So when xmitgcm parses the available diagnostics file, this fix just sets the number of levels to np.NaN if there's 1,000 or more vertical levels for the given diagnostic. The only place this variable seems to be subsequently used is when using the layers package. There's already a warning in place for if the number of layers cannot be inferred and I think this should trigger that warning as required. Short of refactoring the MITgcm code which writes the available_diagnostics.log file I don't think there's any other way to infer the number of levels available.

timothyas commented 2 years ago

Ah, I see now, thanks for that explanation. This does only seem to impact the layers package, but I agree with how you wrote it. That is, I think it's better to catch the error and have levs set before going into the subsequent if/else tree so that the code is flexible for when levs==1 or in the case you're catching when np.isnan(levs).

Test looks good to me too. I say LGTM, and will merge early tomorrow unless there is any disagreement on that.

timothyas commented 2 years ago

Thanks for your efforts @fraserwg!