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

Turn all Meteo models into "real" submodels #124

Closed
tyralla opened this issue Dec 20, 2023 · 6 comments
Closed

Turn all Meteo models into "real" submodels #124

tyralla opened this issue Dec 20, 2023 · 6 comments

Comments

@tyralla
Copy link
Member

tyralla commented Dec 20, 2023

This is a follow-up to #81, which dealt with extracting the calculation of global radiation and sunshine duration from larger models. Unfortunately, we did so before introducing the submodel concept. So, our HydPy-Meteo submodels currently only work as stand-alone models, not as submodels in the sense of #90. This is the reason for the unfavourable situation that, for example, HydPy-L uses HydPy-MORSIM as a "real" submodel, but HydPy-MORSIM needs to be connected to HydPy-Meteo_V1 via the old node-based mechanism.

I hope we will not require many discussions because the main functionalities of all Meteo models are already available. The only problematic point I see coming is that one might wish to use the same Meteo submodel instance twice, for example, for calculating global radiation for the snow routines of HydPy-L and the evaporation submodel used by HydPy-L, which is currently only possible when using the old node-based mechanism.

@tyralla
Copy link
Member Author

tyralla commented Jan 11, 2024

This is my first draft of the interface:

class RadiationModel_V1(modeltools.SubmodelInterface):
    """Simple interface for determining all data in one step."""

    @modeltools.abstractmodelmethod
    def determine_data(self) -> None:
        """Read and calculate all eventually required data."""

    @modeltools.abstractmodelmethod
    def get_potentialsunshineduration(self) -> float:
        """Get the potential sunshine duration in h."""

    @modeltools.abstractmodelmethod
    def get_sunshineduration(self) -> float:
        """Get the sunshine duration in h."""

    @modeltools.abstractmodelmethod
    def get_clearskysolarradiation(self) -> float:
        """Get the clear sky solar radiation in W/m²."""

    @modeltools.abstractmodelmethod
    def get_globalradiation(self) -> float:
        """Get the global radiation in W/m²."""

I think it should be only a little effort to let all submodels support this interface.

@sivogel: The get methods would cover all currently relevant cases, wouldn't they?

@tyralla
Copy link
Member Author

tyralla commented Jan 11, 2024

Here I try to list all models that require such data as input and hence should be able to use the new interface:

  • lland_v3: possible sunshine duration, sunshine duration, global radiation
  • lland_v4: possible sunshine duration, sunshine duration, global radiation
  • evap_fao56: clear sky solar radiation, global radiation
  • evap_tw2002: global radiation
  • evap_morsim: possible sunshine duration, sunshine duration, global radiation
  • evap_pet_ambav1: possible sunshine duration, sunshine duration, global radiation

@sivogel
Copy link
Member

sivogel commented Jan 11, 2024

This is my first draft of the interface:

class RadiationModel_V1(modeltools.SubmodelInterface):
    """Simple interface for determining all data in one step."""

    @modeltools.abstractmodelmethod
    def determine_data(self) -> None:
        """Read and calculate all eventually required data."""

    @modeltools.abstractmodelmethod
    def get_potentialsunshineduration(self) -> float:
        """Get the potential sunshine duration in h."""

    @modeltools.abstractmodelmethod
    def get_sunshineduration(self) -> float:
        """Get the sunshine duration in h."""

    @modeltools.abstractmodelmethod
    def get_clearskysolarradiation(self) -> float:
        """Get the clear sky solar radiation in W/m²."""

    @modeltools.abstractmodelmethod
    def get_globalradiation(self) -> float:
        """Get the global radiation in W/m²."""

I think it should be only a little effort to let all submodels support this interface.

@sivogel: The get methods would cover all currently relevant cases, wouldn't they?

I agree with you. These are all the use cases that I can think of.

@tyralla
Copy link
Member Author

tyralla commented Jan 11, 2024

The list above reveals we currently must serve three data requirement combinations:

  1. global radiation
  2. clear sky solar radiation, global radiation
  3. possible sunshine duration, sunshine duration, global radiation

How do we supply measured or preprocessed data?

It does not seem like a straightforward idea to define individual IO interfaces and submodels for each property (meteo_potsun_io, meteo_sun_io, meteo_potglob_io, meteo_glob_io) because then, for example, evap_fao56 would require one submodel when using meteo_v001 for calculating its data on-the-fly but two submodels (meteo_potglob_io and meteo_glob_io) when using preprocessed data.

The obvious solution seems to define three interfaces and submodels (meteo_glob_io, meteo_potglob_glob_io, and meteo_potsun_sun_glob_io). I do not think we would have to support many more combinations soon, but that is a little ugly. Also, it would make it impossible to combine freshly calculated and preprocessed data (though I doubt this feature is necessary).

My first impression is that the second solution causes less work and hopefully fewer ambiguities.

@tyralla
Copy link
Member Author

tyralla commented Jan 24, 2024

I hope we will not require many discussions because the main functionalities of all Meteo models are already available. The only problematic point I see coming is that one might wish to use the same Meteo submodel instance twice,

All radiation-related Meteo models are now "true" submodels and can be used by all main models listed above. For this purpuse, I had to make meteo_v003 and meteo_v004 also calculate ClearSkySolarRadiation. The found solution leads to reasonable results in all currently available integration tests. However, evaluations based on practical applications are still outstanding.

So, it is time to focus on the mentioned problem of the need (or, at least, the strong desire) to use the same Meteo submodel instance twice. It does not seem to make any sense to support the with statement because one should not change the configuration of an already prepared submodel instance in a control file as it would overwrite previous definitions. So, I propose to use the "submodel adder" methods like "normal" methods when adding already prepared submodel instances:

from hydpy.models.lland_v3 import *
...
with model.add_radiationmodel_v1("meteo_v003") as meteo_v003:
    ...
with model.add_aetmodel_v1("evap_morsim"):
    ...
    model.add_radiationmodel_v1(meteo_v003)

Second, I suggest adding the class ReusableMethod. This class should be the base of all "hydrological" methods of a submodel, such as meteo_v003, that need not or must not be called twice for the same simulation step. "reusable" here means that the method's results can reused, so that the calculation does not need to be repeated.

Not all submodels are sharable, and model users might find it hard to see which one is. Hence, my third suggestion is to introduce SharableSubmodelInterface. Model developers should derive a submodel from this class if convinced it is sharable. HydPy (and Mypy) can automatically check if someone tries to use a submodel twice that is not considered sharable.

I first favoured the name ReusableSubmodelInterface because it is similar to ReusableMethod, but SharableSubmodelInterface seems clearer.

The pure-Python prototype is already working, except for the automatic writing of control files. If there are no alternative ideas, I will start working on this and the required Cython functionalities soon.

@tyralla
Copy link
Member Author

tyralla commented Jan 27, 2024

Cythonized models now work, too.

I also added the repeat_sharedmodels option to the find_submodels method to avoid many duplicate method calls in 0b93463. There are still some duplicate calls when find_submodels cannot be used (mostly during simulation), but this should be only a performance issue, and it's currently unclear if putting effort into this is worth it. Maybe we can rethink this when we implement more "sharable models".

Hence, I consider this issue complete and merge all new features into the master branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants