From cafa59e0c8ad4a161db41e6dbf150c44cdb9b707 Mon Sep 17 00:00:00 2001 From: Francesco Nattino Date: Mon, 18 May 2026 12:02:08 +0200 Subject: [PATCH 1/8] keep crop instance alive for all simulation --- src/diffwofost/physical_models/engine.py | 35 +++++++----------------- 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/src/diffwofost/physical_models/engine.py b/src/diffwofost/physical_models/engine.py index 8657386..dc3bba3 100644 --- a/src/diffwofost/physical_models/engine.py +++ b/src/diffwofost/physical_models/engine.py @@ -129,12 +129,19 @@ def setup( self.weatherdataprovider = weatherdataprovider self.drv = self._get_driving_variables(self.day) + # Call AgroManagement module for management actions at initialization + self.agromanager(self.day, None) + # Component for simulation of soil processes if self.mconf.SOIL is not None: self.soil = self.mconf.SOIL(self.day, self.kiosk, parameterprovider) - - # Call AgroManagement module for management actions at initialization - self.agromanager(self.day, self.drv) + if self.mconf.CROP is not None: + crop_kwargs = {} + if self.mconf.CROP_COMPONENTS: + crop_kwargs["component_overrides"] = self.mconf.CROP_COMPONENTS + self.crop = self.mconf.CROP( + self.day, self.kiosk, self.parameterprovider, shape=self._shape, **crop_kwargs + ) # Calculate initial rates self.calc_rates(self.day, self.drv) @@ -159,25 +166,9 @@ def _on_CROP_START( """ self.logger.debug(f"Received signal 'CROP_START' on day {day}") - if self.crop is not None: - raise RuntimeError( - "A CROP_START signal was received while self.cropsimulation still holds a valid " - "cropsimulation object. It looks like you forgot to send a CROP_FINISH signal with " - "option crop_delete=True" - ) - self.parameterprovider.set_active_crop( crop_name, variety_name, crop_start_type, crop_end_type ) - crop_kwargs = {"shape": self._shape} - if self.mconf.CROP_COMPONENTS: - crop_kwargs["component_overrides"] = self.mconf.CROP_COMPONENTS - self.crop = self.mconf.CROP( - day, - self.kiosk, - self.parameterprovider, - **crop_kwargs, - ) def _finish_cropsimulation(self, day): """Finalize and optionally delete the active crop simulation. @@ -190,12 +181,6 @@ def _finish_cropsimulation(self, day): self.crop.finalize(day) self._save_summary_output() - if self.flag_crop_delete: - self.flag_crop_delete = False - self.crop._delete() - self.crop = None - gc.collect() - def _get_params_shape(parameterprovider): """Infer the common tensor batch shape from a parameter provider. From 4fa78c5577520095d9446cd7841aee37cf0ef23b Mon Sep 17 00:00:00 2001 From: Francesco Nattino Date: Mon, 18 May 2026 14:59:52 +0200 Subject: [PATCH 2/8] allow parameter provider to be a dict --- src/diffwofost/physical_models/engine.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/diffwofost/physical_models/engine.py b/src/diffwofost/physical_models/engine.py index dc3bba3..239499c 100644 --- a/src/diffwofost/physical_models/engine.py +++ b/src/diffwofost/physical_models/engine.py @@ -6,6 +6,7 @@ """ import gc +from collections.abc import MutableMapping from pathlib import Path import torch from pcse import signals @@ -26,6 +27,7 @@ class Engine(PcseEngine): """ mconf = Instance(Configuration) + parameterprovider = Instance(MutableMapping) def __init__( self, @@ -166,9 +168,10 @@ def _on_CROP_START( """ self.logger.debug(f"Received signal 'CROP_START' on day {day}") - self.parameterprovider.set_active_crop( - crop_name, variety_name, crop_start_type, crop_end_type - ) + if hasattr(self.parameterprovider, "set_active_crop"): + self.parameterprovider.set_active_crop( + crop_name, variety_name, crop_start_type, crop_end_type + ) def _finish_cropsimulation(self, day): """Finalize and optionally delete the active crop simulation. @@ -200,7 +203,7 @@ def _get_params_shape(parameterprovider): ValueError: If tensor parameters do not share a common shape. """ shape = () - for paramname in parameterprovider._unique_parameters: + for paramname in parameterprovider.keys(): param = parameterprovider[paramname] if isinstance(param, torch.Tensor): # We need to drop the last dimension from the Afgen table parameters From b88979ee7bc0f773cb7ee0e51a08ca6c38cf928a Mon Sep 17 00:00:00 2001 From: Francesco Nattino Date: Mon, 18 May 2026 16:39:10 +0200 Subject: [PATCH 3/8] fix __iter__ method of parameter provider --- .../physical_models/parameter_providers.py | 12 ++++++++++++ src/diffwofost/physical_models/utils.py | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 src/diffwofost/physical_models/parameter_providers.py diff --git a/src/diffwofost/physical_models/parameter_providers.py b/src/diffwofost/physical_models/parameter_providers.py new file mode 100644 index 0000000..0f861d8 --- /dev/null +++ b/src/diffwofost/physical_models/parameter_providers.py @@ -0,0 +1,12 @@ +from pcse.base.parameter_providers import ParameterProvider as PcseParameterProvider + + +class ParameterProvider(PcseParameterProvider): + """Temporary implementation of PCSE's ParameterProvider. + + Fixes the `__iter__` method in order to allow for access via dict-like `.items()`, `.keys()`, + and `.values()`. + """ + + def __iter__(self): + return iter(self._unique_parameters) diff --git a/src/diffwofost/physical_models/utils.py b/src/diffwofost/physical_models/utils.py index 562bfbc..c46e1f5 100644 --- a/src/diffwofost/physical_models/utils.py +++ b/src/diffwofost/physical_models/utils.py @@ -15,7 +15,6 @@ import torch import yaml from pcse import signals -from pcse.base.parameter_providers import ParameterProvider from pcse.base.weather import WeatherDataContainer from pcse.base.weather import WeatherDataProvider from pcse.settings import settings @@ -23,6 +22,7 @@ from pcse.util import doy from diffwofost.physical_models.config import ComputeConfig from diffwofost.physical_models.engine import Engine +from diffwofost.physical_models.parameter_providers import ParameterProvider logging.disable(logging.CRITICAL) From b221fe0e884c4ac7043f493e996b631f70a68182 Mon Sep 17 00:00:00 2001 From: Francesco Nattino Date: Mon, 18 May 2026 17:14:24 +0200 Subject: [PATCH 4/8] fix signals and flags --- src/diffwofost/physical_models/engine.py | 20 +++++++++++++----- tests/physical_models/test_engine.py | 26 +----------------------- 2 files changed, 16 insertions(+), 30 deletions(-) diff --git a/src/diffwofost/physical_models/engine.py b/src/diffwofost/physical_models/engine.py index 239499c..e0f2fba 100644 --- a/src/diffwofost/physical_models/engine.py +++ b/src/diffwofost/physical_models/engine.py @@ -73,7 +73,6 @@ def _reset_runtime_state(self): self.flag_terminate = False self.flag_crop_finish = False self.flag_crop_start = False - self.flag_crop_delete = False self.flag_output = False self.flag_summary_output = False @@ -152,7 +151,7 @@ def setup( def _on_CROP_START( self, day, crop_name=None, variety_name=None, crop_start_type=None, crop_end_type=None ): - """Instantiate the crop component after a crop-start signal. + """Set active crop parameters for providers that support that. Args: day: Current simulation day. @@ -162,9 +161,6 @@ def _on_CROP_START( signal. crop_start_type: Crop start mode used by the parameter provider. crop_end_type: Crop end mode used by the parameter provider. - - Raises: - RuntimeError: If a crop component is already active. """ self.logger.debug(f"Received signal 'CROP_START' on day {day}") @@ -173,6 +169,20 @@ def _on_CROP_START( crop_name, variety_name, crop_start_type, crop_end_type ) + def _on_CROP_FINISH(self, day): + """Flag finishing of the crop simulation. + + The flag is needed because finishing the crop simulation is deferred to + the correct place in the processing loop and is done by the routine + _finish_cropsimulation(). + + Differently from PCSE, diffWOFOST does not delete the crop simulation instance. + + Args: + day: Current simulation day. + """ + self.flag_crop_finish = True + def _finish_cropsimulation(self, day): """Finalize and optionally delete the active crop simulation. diff --git a/tests/physical_models/test_engine.py b/tests/physical_models/test_engine.py index b0c818c..2828b1c 100644 --- a/tests/physical_models/test_engine.py +++ b/tests/physical_models/test_engine.py @@ -159,7 +159,7 @@ def test_on_crop_start_raises_when_crop_is_already_active(self): with pytest.raises(RuntimeError, match="A CROP_START signal was received"): engine._on_CROP_START(engine.day) - def test_finish_cropsimulation_deletes_crop_when_requested(self): + def test_finish_cropsimulation_does_not_delete_crop(self): _, (crop_model_params_provider, weather_data_provider, agro_management_inputs, _) = ( _get_engine_inputs() ) @@ -169,35 +169,11 @@ def test_finish_cropsimulation_deletes_crop_when_requested(self): crop.finalize = Mock() crop._delete = Mock() engine.flag_crop_finish = True - engine.flag_crop_delete = True engine._save_summary_output = Mock() engine._finish_cropsimulation(date(2000, 1, 1)) assert engine.flag_crop_finish is False - assert engine.flag_crop_delete is False - crop.finalize.assert_called_once_with(date(2000, 1, 1)) - engine._save_summary_output.assert_called_once_with() - crop._delete.assert_called_once_with() - assert engine.crop is None - - def test_finish_cropsimulation_keeps_crop_when_not_deleting(self): - _, (crop_model_params_provider, weather_data_provider, agro_management_inputs, _) = ( - _get_engine_inputs() - ) - engine = Engine(config=config) - engine.setup(crop_model_params_provider, weather_data_provider, agro_management_inputs) - crop = engine.crop - crop.finalize = Mock() - crop._delete = Mock() - engine.flag_crop_finish = True - engine.flag_crop_delete = False - engine._save_summary_output = Mock() - - engine._finish_cropsimulation(date(2000, 1, 1)) - - assert engine.flag_crop_finish is False - assert engine.flag_crop_delete is False crop.finalize.assert_called_once_with(date(2000, 1, 1)) engine._save_summary_output.assert_called_once_with() crop._delete.assert_not_called() From 91ab41ace89af4d96a03007c8f6a4535f9cc48fe Mon Sep 17 00:00:00 2001 From: Francesco Nattino Date: Wed, 10 Jun 2026 11:10:00 +0200 Subject: [PATCH 5/8] move crop instance creation to function --- src/diffwofost/physical_models/engine.py | 31 +++++++++++++++--------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/diffwofost/physical_models/engine.py b/src/diffwofost/physical_models/engine.py index a53442c..a84ef32 100644 --- a/src/diffwofost/physical_models/engine.py +++ b/src/diffwofost/physical_models/engine.py @@ -145,18 +145,9 @@ def setup( # Component for simulation of soil processes if self.mconf.SOIL is not None: self.soil = self.mconf.SOIL(self.day, self.kiosk, parameterprovider) + # Component for crop simulation if self.mconf.CROP is not None: - crop_args = [self.day, self.kiosk, self.parameterprovider] - crop_kwargs = {"shape": self._shape} - - if self.mconf.CROP_NN_MODEL is not None: - # crop_nn_model initialize doesnot accpet parameterprovider - crop_args = [self.day, self.kiosk, self.mconf.CROP_NN_MODEL] - - if self.mconf.CROP_COMPONENTS: - crop_kwargs["component_overrides"] = self._components_overrides - - self.crop = self.mconf.CROP(*crop_args, **crop_kwargs) + self._create_crop(self.day) # Calculate initial rates self.calc_rates(self.day, self.drv) @@ -183,6 +174,24 @@ def _on_CROP_START( crop_name, variety_name, crop_start_type, crop_end_type ) + def _create_crop(self, day): + """Setup crop model instance. + + Args: + day: Current simulation day + """ + crop_args = [day, self.kiosk, self.parameterprovider] + crop_kwargs = {"shape": self._shape} + + if self.mconf.CROP_NN_MODEL is not None: + # crop_nn_model initialize does not accept parameterprovider + crop_args = [day, self.kiosk, self.mconf.CROP_NN_MODEL] + + if self.mconf.CROP_COMPONENTS: + crop_kwargs["component_overrides"] = self._components_overrides + + self.crop = self.mconf.CROP(*crop_args, **crop_kwargs) + def _on_CROP_FINISH(self, day): """Flag finishing of the crop simulation. From 393b7aaa0f30385bb7e40a36fe1d9ddeaf26bc0e Mon Sep 17 00:00:00 2001 From: Francesco Nattino Date: Wed, 10 Jun 2026 11:10:17 +0200 Subject: [PATCH 6/8] fix tests on engine --- tests/physical_models/test_engine.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/tests/physical_models/test_engine.py b/tests/physical_models/test_engine.py index 0b0ac4c..d2061d7 100644 --- a/tests/physical_models/test_engine.py +++ b/tests/physical_models/test_engine.py @@ -151,16 +151,6 @@ def test_setup_initializes_soil_component_when_configured(self): assert returned_engine is engine assert engine.soil is not None - def test_on_crop_start_raises_when_crop_is_already_active(self): - _, (crop_model_params_provider, weather_data_provider, agro_management_inputs, _) = ( - _get_engine_inputs() - ) - engine = Engine(config=config) - engine.setup(crop_model_params_provider, weather_data_provider, agro_management_inputs) - - with pytest.raises(RuntimeError, match="A CROP_START signal was received"): - engine._on_CROP_START(engine.day) - def test_finish_cropsimulation_does_not_delete_crop(self): _, (crop_model_params_provider, weather_data_provider, agro_management_inputs, _) = ( _get_engine_inputs() @@ -234,6 +224,6 @@ def initialize(self, day, kiosk, nn_model, shape=None): engine.parameterprovider = ParameterProvider() engine.kiosk = VariableKiosk() engine._shape = () - engine._on_CROP_START(date(2000, 1, 1)) + engine._create_crop(date(2000, 1, 1)) assert engine.mconf.CROP_NN_MODEL == nn_model assert engine.crop._initialized is True From 2b9cf77f85f03a2e9bf4811a3328e96b49046d93 Mon Sep 17 00:00:00 2001 From: Francesco Nattino Date: Wed, 10 Jun 2026 11:57:33 +0200 Subject: [PATCH 7/8] add new tests --- tests/physical_models/test_engine.py | 20 +++++++++++++++++++ .../test_parameter_providers.py | 12 +++++++++++ 2 files changed, 32 insertions(+) create mode 100644 tests/physical_models/test_parameter_providers.py diff --git a/tests/physical_models/test_engine.py b/tests/physical_models/test_engine.py index d2061d7..2cbf151 100644 --- a/tests/physical_models/test_engine.py +++ b/tests/physical_models/test_engine.py @@ -4,7 +4,9 @@ import pytest import torch from pcse.base import ParameterProvider +from pcse.base import SimulationObject from pcse.base.variablekiosk import VariableKiosk +from diffwofost.physical_models.base import TensorParamTemplate from diffwofost.physical_models.config import Configuration from diffwofost.physical_models.crop.phenology import DVS_Phenology from diffwofost.physical_models.crop.wofost72 import Wofost72 @@ -13,6 +15,7 @@ from diffwofost.physical_models.soil.classic_waterbalance import WaterbalancePP from diffwofost.physical_models.test import get_test_data from diffwofost.physical_models.test import prepare_engine_input +from diffwofost.physical_models.traitlets import Tensor from . import phy_data_folder config = Configuration( @@ -52,6 +55,14 @@ def set_active_crop(self, *args): self.active_crop_args = args +class DummyCropModel(SimulationObject): + class Parameters(TensorParamTemplate): + A = Tensor(-99, dtype=int) + + def initialize(self, day, kiosk, parvalues, shape): + self.params = self.Parameters(parvalues, shape=shape) + + @pytest.mark.usefixtures("fast_mode") class TestEngine: def test_engine_requires_config(self): @@ -227,3 +238,12 @@ def initialize(self, day, kiosk, nn_model, shape=None): engine._create_crop(date(2000, 1, 1)) assert engine.mconf.CROP_NN_MODEL == nn_model assert engine.crop._initialized is True + + def test_engine_accept_dict_as_parameter_provider(self): + engine = Engine(config=Configuration(CROP=DummyCropModel)) + engine.parameterprovider = {"A": 10} + engine.kiosk = VariableKiosk() + engine._shape = () + engine._create_crop(date(2000, 1, 1)) + assert isinstance(engine.crop.params.A, torch.Tensor) + assert engine.crop.params.A == 10 diff --git a/tests/physical_models/test_parameter_providers.py b/tests/physical_models/test_parameter_providers.py new file mode 100644 index 0000000..c064895 --- /dev/null +++ b/tests/physical_models/test_parameter_providers.py @@ -0,0 +1,12 @@ +from diffwofost.physical_models.parameter_providers import ParameterProvider + + +class TestParameterProvider: + def test_parameter_provider_supports_dict_methods(self): + p = ParameterProvider( + sitedata={"A": 0}, timerdata={"B": 1}, soildata={"C": 2}, cropdata={"D": 3} + ) + p.set_override("E", 4, check=False) + assert len(p.items()) == len(p.keys()) == len(p.values()) == 5 + assert set(p.keys()) == set("ABCDE") + assert set(p.values()) == set(range(5)) From a6029640d5e9f5b69d8dff58625d98d6c81204cb Mon Sep 17 00:00:00 2001 From: Francesco Nattino Date: Fri, 12 Jun 2026 13:53:04 +0200 Subject: [PATCH 8/8] add reference to PR --- src/diffwofost/physical_models/parameter_providers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/diffwofost/physical_models/parameter_providers.py b/src/diffwofost/physical_models/parameter_providers.py index 0f861d8..204acc1 100644 --- a/src/diffwofost/physical_models/parameter_providers.py +++ b/src/diffwofost/physical_models/parameter_providers.py @@ -5,7 +5,7 @@ class ParameterProvider(PcseParameterProvider): """Temporary implementation of PCSE's ParameterProvider. Fixes the `__iter__` method in order to allow for access via dict-like `.items()`, `.keys()`, - and `.values()`. + and `.values()`. Could be dropped when https://github.com/ajwdewit/pcse/pull/121 is merged. """ def __iter__(self):