Repository navigation
Conversation
| class NaNPolicyMixin(Base): | ||
| """Optional rejection or replacement of missing data.""" | ||
|
|
||
| reject_nan: bool = Field( |
There was a problem hiding this comment.
We now have the option to reject NaNs and raise an error if any are found
| @model_validator(mode="after") | ||
| def validate_nan_policy(self) -> "NaNPolicyMixin": | ||
| """Reject filling when NaNs must raise an error.""" | ||
| if self.reject_nan and self.fill_value is not None: |
There was a problem hiding this comment.
Having a fill_value doesn't make sense if NaNs are to be raised
| @model_validator(mode="after") | ||
| def validate_fill_value(self) -> "DropoutMixin": | ||
| """Require a replacement value when observations can be dropped out.""" | ||
| if self.dropout_minute_probabilities is not None and self.reject_nan: |
There was a problem hiding this comment.
We can't use dropout if we won't accept NaNs
| datasets_dict, location_ids, config, time_periods, | ||
| ) | ||
| add_source_coordinates(self.locations, datasets_dict) | ||
| datasets_dict = reduce_spatial_extent_of_datasets(datasets_dict, self.locations, config) |
There was a problem hiding this comment.
Now we pre-slice the spatial area in both PVNet datasets
| validate_and_fill_nans( | ||
| datasets_dict["generation_target"], | ||
| fill_value=None, | ||
| reject_nan=True, |
There was a problem hiding this comment.
No NaNs are allowed in the targets for now. I can see a world where do allow this and only backpropagate where the ground truths are not NaN. We do that for cloudcasting, but it would take a lot of work. So for now we raise for NaNs in the target generation
| raise ValueError("At least one accumulated channel must be requested") | ||
|
|
||
| all_channels = da["channel"].values.tolist() | ||
| accum_channel_inds = [all_channels.index(c) for c in accum_channels] |
There was a problem hiding this comment.
This will raise if a channel in accum_channels is missing
|
|
||
| def test_apply_dropout_to_datasets(config_filename): | ||
| config = load_yaml_configuration(config_filename) | ||
| config.nwp = None |
There was a problem hiding this comment.
We now look at the config rather than the dataset dict to do infilling NaNs. We didn't have NWP in the dataset dict use here but we hadn't set this to None. So this is just a little correction
…d slightly improve spatial slice efficiency
|
Looking good. One thing, sorry if being pedantic. In diff_channels a missing channel gives default “‘x’ is not in list” error. Maybe a better error message there? Thanks! |
2 participants
Pull Request
Description
This PR wraps together a couple of unrelated things
There are two very small pieces ...
And one bigger piece in updating how we NaNs
Previous we used
nan_to_numwhich has a side effect of converting infs to the largest of smallest value in the dtype. This conversion was unintended. For now I've added the functionality to scan for infinite values and raise an error if any are found. I cannot think of a use-case where infinite values would be intentional.I've also now updated how we handle NaNs and this can be configured per source, with different fill values and different policies. We may want to reject NaNs in NWPs for example but have to accept and fill them in satellite data.
Checklist: