Facilitating support for tensor weather data

[fnattino]

Currently PCSE uses custom objects to provide weather data (WeatherDataProvider and WeatherDataContainer). In order to support tensor data, I was initially thinking to implement a new class, as e.g. sketched in #113. However, this approach has the disadvantage that everyone needs to write custom derived subclasses for any new data structure used for the actual data (e.g. for a pandas DataFrame, for a xarray Dataset, etc.). Also, it might be challenging to design a generic class that would be suitable for different use cases (small and large datasets).

But what if we would simply ask for a generic iterator as input to the engine? We could only expect the iterator to return a dictionary of tensors. Just to give a practical example, given a dataframe where each column represent a weather variable and each row a day, one could build the data provider as:

def iterate(df):
    cols = {
        col: torch.tensor(df[col].to_numpy())
        for col in df.columns
    }
    for n in range(len(df)):
        yield {k: v[n] for k, v in cols.items()}

engine.setup(..., weatherdataprovider=iterate(df), ...)

I am sketching here some changes to enable this, it is actually not too big changes. The biggest difference is that one would need to make sure the weatherdataprovider returns weather data in the same order as expected by the simulation.

What do you think @SarahAlidoost ?

[SarahAlidoost]

Currently PCSE uses custom objects to provide weather data (WeatherDataProvider and WeatherDataContainer). In order to support tensor data, I was initially thinking to implement a new class, as e.g. sketched in #113. However, this approach has the disadvantage that everyone needs to write custom derived subclasses for any new data structure used for the actual data (e.g. for a pandas DataFrame, for a xarray Dataset, etc.). Also, it might be challenging to design a generic class that would be suitable for different use cases (small and large datasets).

@fnattino thanks! this looks like a good idea 👍

But what if we would simply ask for a generic iterator as input to the engine? We could only expect the iterator to return a dictionary of tensors.

That's the right track. The Engine and Crop object accepts weather data as a dictionary of tensors, similar to parameters. How to get that dictionary is a data-processing concern and happens outside the engine.

I am sketching here some changes to enable this, it is actually not too big changes. The biggest difference is that one would need to make sure the weatherdataprovider returns weather data in the same order as expected by the simulation.

That's right! we can also implement some checks in init of Engine to make sure weather data is as expected, like checking format and shapes.

What do you think @SarahAlidoost ?

[fnattino]

This PR should be now ready to be reviewed.

This introduced a breaking change in the data structure required to provide weather data to diffWOFOST. While till now we used the WeatherDataProvider and WeatherDataContainer from PCSE, with this PR diffWOFOST expects an iterator that returns a dictionary of tensors for weather data. This way we can be flexible in the way we provide data to the model (differently-sized usecases will have different needs in terms e.g. of whether all data should be already in memory or read while processing). The dimensionality of the input weather data is compared with the one of the model parameters from the engine.

For "standard" use, we provide the to_weather_data_iterator utility function that generates the iterator required by diffWOFOST from a pandas DataFrame or xarray Dataset. Different behaviours with respect to the previous WeatherDataProvider:

• range validity checks are vectorized (and can be skipped).
• we allow for a subset of weather variables (e.g. precipitation data is not needed to run only phenology).

[sonarqubecloud]

Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
96.2% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[SarahAlidoost]

Suggested change

	# These are the weather variables recognized by diffWOFOST internally, along
	# with their units and valid ranges.

[SarahAlidoost]

Could the util function _broadcast_to be used instead of defining new function broadcast in the test here?

[SarahAlidoost]

Why we need to remove this test? is this because of weatherprovider? If we remove this test, and something is changed in diffwofost differentiable modules, we cannot validate the results. We can make this work by implementation that I suggested here. I checked the implementation for this class.

[SarahAlidoost]

we need to keep this class to be able to check whether the results of differentiable modules are the same as those generated by pcse, see the tests that you removed them. A suggestion for this to work:

class WeatherDataContainerTestHelper(WeatherDataContainer):
    """A helper class for creating WeatherDataContainer instances for YAML tests."""
    def __getitem__(self, key):
        return getattr(self, key)


class WeatherDataProviderTestHelper(WeatherDataProvider):
    """It stores the weatherdata contained within the YAML tests."""

    def __init__(self, yaml_weather, meteo_range_checks=True):
        super().__init__()
        # This is a temporary workaround. The `METEO_RANGE_CHECKS` logic in
        # `__setattr__` method in `WeatherDataContainer` is not vector compatible
        # yet. So we can disable it here when creating the `WeatherDataContainer`
        # instances with arrays.
        settings.METEO_RANGE_CHECKS = meteo_range_checks
        for weather in yaml_weather:
            weather_inputs = {k: v for k, v in weather.items() if k != "SNOWDEPTH"}
            wdc = WeatherDataContainerTestHelper(**weather_inputs)
            self._store_WeatherDataContainer(wdc, wdc.DAY)

and add this to prepare_engine_input function:

    if test_pcse:
        weather_data_provider = WeatherDataProviderTestHelper(
            test_data["WeatherVariables"], meteo_range_checks=meteo_range_checks
        )
    else:

        weather_data = pd.DataFrame(test_data["WeatherVariables"])
        if "DTEMP" not in weather_data.columns:
            weather_data["DTEMP"] = (weather_data["TEMP"] + weather_data["TMAX"]) / 2.0

        # create a list out of the iterator, so that the weather data can be reused in several tests
        weather_data_provider = list(to_weather_data_iterator(weather_data, check=meteo_range_checks))

later in tests, the argument test_pcse can be added to only tests including patching.

[SarahAlidoost]

@fnattino clean and nice implementation, thanks! 👍 there is only one concern about the test including patching which are removed. See my comments. We can also discuss it offline.