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

Possible EMOS amendments #11

Open
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

gavinevans
Copy link
Owner

@gavinevans gavinevans commented Jun 14, 2021

Description
A branch containing some possible EMOS amendments. Tests have not been updated. or added to support the new features.

This PR is only to help facilitate an initial look and will not be merged.

Testing:

  • Ran tests and they passed OK
  • Added new tests for the new feature(s)

predictor,
).x.astype(np.float32)
)
for index, (truth_slice, fv_slice) in enumerate(zip(truth.slices_over(sindex), forecast_var.slices_over(sindex))):

Choose a reason for hiding this comment

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

Preprocessing of arrays for additional predictors before passing to minimising,

forecast_predictor_data = flatten_ignoring_masked_data(
forecast_predictor.data
)
fp_data_list = []

Choose a reason for hiding this comment

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

More preprocessing for additional predictors

initial_guess[-2],
initial_guess[-1],
)
a_b = np.array([a] + b.tolist(), dtype=np.float64)

if predictor.lower() == "mean":

Choose a reason for hiding this comment

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

Again additional predictors and more initiall guesses

forecast_predictor_flattened = flatten_ignoring_masked_data(
forecast_predictor
forecast_predictor, preserve_leading_dimension=True

Choose a reason for hiding this comment

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

All additional predictor related

for (truths_slice, fp_slice) in zip(
truths.slices_over(index), forecast_predictor.slices_over(index)
):
for truths_slice in truths.slices_over(index):

Choose a reason for hiding this comment

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

Similar to above, pre processing of additional predictors

"beta coefficients in order to create a calibrated forecast.")
raise ValueError(msg)

if self.standardise_cubelist:

Choose a reason for hiding this comment

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

All the equations in this section to do with applying coefficients if the standardisation was used when calculating the coefficients are now correct to the best of my understanding.

return std_forecast, std_truth, forecast_mean, forecast_sd, truth_mean, truth_sd


def standardise_forecasts(historic_forecasts, hf_coords=["realization", "time"]):

Choose a reason for hiding this comment

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

This function and the one below aren't currently used.


if truths.coords("time", dim_coords=True):
truth_mean = truths.collapsed(truth_coords, nanmean)
else:

Choose a reason for hiding this comment

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

Why are these else statements needed?

Choose a reason for hiding this comment

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

This is dealing with a spin up problem. It will not impact the testing we are currently doing as we should always have more than one truth. For now it is probably best to fail if this else statement is reached.

Choose a reason for hiding this comment

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

Now done

coord_values={y_name: lambda cell: any(np.isclose(cell.point, truth_slice.coord(axis="y").points)),
x_name: lambda cell: any(np.isclose(cell.point, truth_slice.coord(axis="x").points))})
new_fp_data = []
for fp_cube in forecast_predictors:

Choose a reason for hiding this comment

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

I think I need some help explaining this

Choose a reason for hiding this comment

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

This is for static predictors where they don't change with time. Give them a time coordinate so they match the shape of the other predictor.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I've added a comment to indicate that this is to account for static predictors.

Choose a reason for hiding this comment

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

Thanks

@@ -215,13 +218,38 @@ def filter_non_matching_cubes(
if truth_slice:

Choose a reason for hiding this comment

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

Suggested change
if truth_slice:
if truth_slice and af_slices:
matching_historic_forecasts.append(hf_slice)
matching_truths.append(truth_slice)
matching_additional_fields.extend(af_slices)

Copy link
Owner Author

Choose a reason for hiding this comment

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

I've modified this to avoid the possible issue of truths and historic forecasts being added potentially without a required additional predictor.

@gavinevans gavinevans force-pushed the impro_2102_additional_predictors branch from b72eb70 to 4aebecf Compare June 29, 2021 13:41
Copy link

@fionaRust fionaRust left a comment

Choose a reason for hiding this comment

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

I'm now happy that these changes are suitable for use in the EMOS comparison experiments for temperature

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.

2 participants