noaa-ocs-modeling / EnsemblePerturbation

perturbation of coupled model input over a space of input variables
https://ensembleperturbation.readthedocs.io
Creative Commons Zero v1.0 Universal
7 stars 3 forks source link

Support `scipy>1.12` on macos #153

Open SorooshMani-NOAA opened 3 months ago

SorooshMani-NOAA commented 3 months ago

As also noted in https://github.com/oceanmodeling/adcircpy/issues/190, the latest scipy seems to have some issues on macos. It results in different values. When calculating the perturbation I think we're doing some type of interpolation again, which can be causing the issue I see in the failed test

Pinning the version seems to resolve the issue see https://github.com/noaa-ocs-modeling/EnsemblePerturbation/pull/149/commits/27f63da82363787de88c43d581980c46780bbaff in PR #149

@WPringle what do you think? I'll need to confirm by making sure all tests go through, but for now I don't get the difference in perturbed json file at least (there are other problems I have to solve though!)

WPringle commented 3 months ago

I don't have any issues pinning scipy version if it works well for us and not at risk of being deprecated for a while.

SorooshMani-NOAA commented 3 months ago

OK thanks, can you point out where in the perturbation we use interpolation? Just for the sake of documenting the issue. Hopefully scipy will get a fix later before the pinned version is deprecated ... otherwise we have to find a better test rather than checking against the ref file, since now linux and macos result in slightly different values.

WPringle commented 3 months ago

Not sure, in this code it just uses the interp function in pandas.. https://github.com/noaa-ocs-modeling/EnsemblePerturbation/blob/71804ad4488492471da9971d564aee76e648c94f/ensembleperturbation/perturbation/atcf.py#L1562C3-L1575C22

                    # forecast errors than grow with time based on initial intensity
                    validation_time_errors = DataFrame(
                        data={
                            column: interp(
                                x=validation_hours,
                                xp=historical_forecast_errors.index,
                                fp=historical_forecast_errors.loc[
                                    :, column
                                ].values.quantity.magnitude,
                            )
                            for column in historical_forecast_errors.columns
                        },
                        index=validation_hours,
                    )
SorooshMani-NOAA commented 3 months ago

OK thanks, I guess that's it, I'll double check when we want to look into this issue.

SorooshMani-NOAA commented 3 months ago

Note to self, scipy<=0.12 does not support numpy>=2. Is it possible that the difference between linux vs macos is actually something in numpy and not directly in scipy?!