Configure the engine#59
Conversation
There was a problem hiding this comment.
This is just an initial placeholder for the "new" diffwofost engine. Right now it is identical to the pcse.engine.Engine, with exception for the use of the , with exception for the use of the actual Configuration instead of the ConfigurationLoaderVariableKiosk vs the VariableKioskTestHelper (which allows to set in external variables).
| leaf_dynamics_config = Configuration.from_pcse_config_file( | ||
| phy_data_folder / "WOFOST_Leaf_Dynamics.conf" | ||
| ) |
There was a problem hiding this comment.
By initializing this here, we load the model configuration file only once for all the tests in this module. But we could even initialize it in-memory as in:
from diffwofost.physical_models.crop.leaf_dynamics import WOFOST_Leaf_Dynamics
leaf_dynamics_config = Configuration(
CROP=WOFOST_Leaf_Dynamics,
OUTPUT_VARS=["LAI", "TWLV"]
)so we could get rid of the .conf files from the testdata folder. What do you think?
There was a problem hiding this comment.
Actually, we should keep at least one config file in order to test the ability to set up the configuration from a config file - see this test
There was a problem hiding this comment.
Awesome! this is exactly what I had in mind 👍 . We should define the config as an object and remove the config files. It is okay to keep one for test purposes, but let's remove them from crop model tests, like leaf_dynamics, root_dynamics, and phenology, thanks!
|
Just realized tests are failing because of a feature introduced which would require Python >= 3.11: https://github.com/WUR-AI/diffWOFOST/pull/59/files#r2589286305 - what do you think @SarahAlidoost ? |
|
Sorry @SarahAlidoost after requesting your review I have made a couple of extra commits - hope you hadn't start to have a look yet and apologies if you did! This is now complete. |
| # Ignore deprecation warnings from pcse.base.simulationobject | ||
| pytestmark = pytest.mark.filterwarnings("ignore::DeprecationWarning:pcse.base.simulationobject") | ||
|
|
||
| phenology_config = Configuration.from_pcse_config_file(phy_data_folder / "WOFOST_Phenology.conf") |
There was a problem hiding this comment.
let's replace this with the config object. This helps users/other developers to know how to use it.
| # Ignore deprecation warnings from pcse.base.simulationobject | ||
| pytestmark = pytest.mark.filterwarnings("ignore::DeprecationWarning:pcse.base.simulationobject") | ||
|
|
||
| root_dynamics_config = Configuration.from_pcse_config_file( |
|
Thanks for the review @SarahAlidoost - I have:
|
|
2 participants
First (small) bit for #25. With this PR, we allow for the in-memory configuration of the engine, without the need to parse a config file every time the engine is initialized.