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

Storm day restart date #194

Merged
merged 1 commit into from
Oct 19, 2020
Merged

Storm day restart date #194

merged 1 commit into from
Oct 19, 2020

Conversation

scotthavens
Copy link
Contributor

The timezone wasn't being set when using the storm days restart file. Cleaned this up to ensure that we're comparing python datetime objects instead of cftime and pd.Timestep.

@@ -134,28 +134,32 @@ def initialize(self, topo, data, date_time=None):
self._logger.debug('Reading {} from {}'.format(
'storm_days', self.config['storm_days_restart']))
f = nc.Dataset(self.config['storm_days_restart'], 'r')
f.set_always_mask(False)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Never used this. What does it do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

netCDF4 in their infinite wisdom decided to return a numpy masked array all the time, even if there is no fill values. So this returns a normal numpy array which is faster and easier to deal with. The masked arrays are the opposite of what we think them to be where the mask is the values we don't want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

xarray doesn't do this, just saying.... 😉

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ha ... still not sold on Xarray being the all mighty ;-)

I might be misunderstanding this with the description, but is the mask by netCDF saying True when a value is incorrect and 'False` where it is correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha, I know it's not, it's just convenient sometimes.

"When an element of the mask is False, the corresponding element of the associated array is valid and is said to be unmasked. When an element of the mask is True, the corresponding element of the associated array is said to be masked (invalid)."

This has caught us in the past because our convention is that a mask is True where the data is valid.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I can see the argument for that behaviour of netCDF. For instance rasterio follows the same principle: https://rasterio.readthedocs.io/en/latest/topics/masks.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's probably specific to our (IPW and hydrology) representation of what mask is, whether it's inside or outside of a basin.

@jomey
Copy link
Collaborator

jomey commented Oct 16, 2020

Any quick test that can be added?

@scotthavens
Copy link
Contributor Author

Any quick test that can be added?

Probably but we don't have a storm day netcdf that is in the test data. Right now, the test is in AWSM during the restarting tests, which was how this was discovered.

@scotthavens scotthavens merged commit e63bcae into master Oct 19, 2020
@scotthavens scotthavens deleted the storm_days_restart branch August 3, 2021 15:08
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

2 participants