Make the Leaf dynamics differentiable by SarahAlidoost · Pull Request #12 · WUR-AI/diffWOFOST

SarahAlidoost · 2025-08-06T07:22:12Z

relates #8

…al vars

… tdwi

…n leaf_dynamics

cwmeijer · 2025-08-28T14:22:13Z

I've installed the package and ran the tests. All 10 of them pass. So that's a good start. Will look at the actual code next week.

I also see ruff is complaining about ANN101 and ANN102 that seem to be configured somewhere to be ignored, while these rules no longer exist and are from older versions of ruff. I made a PR for this: #14

[ANN101](https://docs.astral.sh/ruff/rules/missing-type-self/) and [ANN102](https://docs.astral.sh/ruff/rules/missing-type-cls/) have been removed and referencing them in the config causes warnings to be printed when ruff is run. See also WUR-AI/diffWOFOST#12 (comment)

cwmeijer

I think this is a great PR! 🎖Very neat!
I made some comments. Some are suggestions for possible improvements and others are more points for you to think about (or discuss together) before you decide to merge.
I also created a PR with a tiny refactoring of a single file in your code. Was easier to do than to explain. See #15 . I think that should be merged into this branch before merging your whole PR into main.
I hope it is helpful for you!

refactor: extract model creation method

SarahAlidoost · 2025-09-01T13:35:01Z

I've installed the package and ran the tests. All 10 of them pass. So that's a good start. Will look at the actual code next week.

I also see ruff is complaining about ANN101 and ANN102 that seem to be configured somewhere to be ignored, while these rules no longer exist and are from older versions of ruff. I made a PR for this: #14

thanks! 👍 we can merge #14

SarahAlidoost · 2025-09-01T13:41:17Z

I think this is a great PR! 🎖Very neat! I made some comments. Some are suggestions for possible improvements and others are more points for you to think about (or discuss together) before you decide to merge. I also created a PR with a tiny refactoring of a single file in your code. Was easier to do than to explain. See #15 . I think that should be merged into this branch before merging your whole PR into main. I hope it is helpful for you!

Thanks for the comments and helpful suggestions. I addressed your comments:

we can merge Remove deprecated linting rules from pyproject.toml #14 🚀
I merged refactor: extract model creation method #15 in this branch. Thanks for the suggestions.
I submitted issue Explore using pytest-subsets package in tests #16 for pytest-subtests pacakge
I created a discussion list in [Backlog Item]: Build a prototype of differentiable wofost72_pp #3, see here
I added two more tests where gradients are calculated numerically i.e. Δloss/Δtdwi and Δloss/Δspan

michielkallenberg · 2025-09-17T07:29:08Z

+        TWLV = Any(default_value=torch.tensor(-99.0, dtype=DTYPE))
+
+    class RateVariables(RatesTemplate):
+        GRLV = Any(default_value=torch.tensor(0.0, dtype=DTYPE))


in original these were initialized at -99.0

You're right. In PCSE, class attributes default to -99.0, but rate variables are initialized to zero, see here. So in practice, those -99.0 values never actually get used.

michielkallenberg · 2025-09-17T07:29:36Z

+        TBASE = Any(default_value=[torch.tensor(-99.0, dtype=DTYPE)])
+        PERDL = Any(default_value=[torch.tensor(-99.0, dtype=DTYPE)])
+        TDWI = Any(default_value=[torch.tensor(-99.0, dtype=DTYPE)])
+        SLATB = AfgenTrait()  # FIXEME


This PR was about two parameters of leaf_dynamics; TDWI and SPAN. In the next iteration, the rest will be fixed.

michielkallenberg · 2025-09-17T07:30:36Z

+                key/value pairs
+        """
+        self.kiosk = kiosk
+        # TODO check if parvalues are already torch.nn.Parameters


Is this TODO still active?

yes, currently, there are no checks on the parameters since we are not using Traits anymore. This should be fixed in the next iterations.

michielkallenberg · 2025-09-17T07:30:41Z

+        # CALCULATE INITIAL STATE VARIABLES
+        # check for required external variables
+        _exist_required_external_variables(self.kiosk)
+        # TODO check if external variables are already torch tensors


Is this TODO still active?

yes, the same as above.

michielkallenberg · 2025-09-17T07:49:01Z

+        self.states.LVAGE = tLVAGE
+
+    @prepare_states
+    def _set_variable_LAI(self, nLAI):  # FIXEME


What is this FIXEME?

This function, as a BMI function, might not be used in any application that requires differentiability. For now, I kept it, but will most probably be removed.

michielkallenberg

Very nice work! Great. I left some minor comments; feel very much free to ignore.

michielkallenberg · 2025-09-17T08:54:55Z

+from pcse.base.parameter_providers import ParameterProvider
+from pcse.engine import Engine
+from pcse.models import Wofost72_PP
+from diffwofost.physical_models.crop.leaf_dynamics import WOFOST_Leaf_Dynamics


, WeatherDataProviderTestHelper

michielkallenberg · 2025-09-17T09:19:38Z

+        span = torch.nn.Parameter(torch.tensor(30, dtype=torch.float64))
+        numerical_grad = calculate_numerical_grad("SPAN", span, "TWLV")  # this is Δloss/Δspan
+
+        assert numerical_grad == 0.0


this test seems not entirely complete

right, it is fixed now.

michielkallenberg · 2025-09-17T09:20:32Z

+        assert_almost_equal(numerical_grad, grads.item(), decimal=3)
+
+
+class TestDiffLeafDynamicsSPAN:


Conssider making this more generic; there is a bit of code repetion.

I see your point. While there’s some duplication, it helps keep each test clear and maintainable on its own. I’d keep them as-is unless in a future iteration, we see a need for reuse.

michielkallenberg · 2025-09-17T09:22:49Z

+        self.TERMINAL_OUTPUT_VARS = []
+
+
+class EngineTestHelper(Engine):


There's a bit of code repetition from the original class.

That's correct. This file will be improved later.

SarahAlidoost · 2025-09-18T11:45:31Z

Very nice work! Great. I left some minor comments; feel very much free to ignore.

Thanks for the review and comments/suggestions.

SarahAlidoost added 18 commits July 31, 2025 13:42

add pcse to dependencies in pyproject.toml

2838a19

add leaf_dynamics script from pcse, add a conf and a check for extern…

b68c0f1

…al vars

add two yaml files containg test data from pcse

9d62f26

setup unit tests for leaf_dynamics

d459bea

make arrays and operations in leaf_dynamic torch compatible

f879876

disable debug logs

ea47ad3

fix parameters in leaf_dyna,ics, fix tests, add tests for gradient of…

5c24a32

… tdwi

refactor tests

7a31a6b

replace a hard threshold with torch.sigmoid when SPAN requires grad i…

09ffb3d

…n leaf_dynamics

add tests for SPAN

e7afb20

change some of the ruff settings

72e2f62

fix some linter errors

c4d0fa9

fix a test

b5f86fd

move model conf to tests folder

960679a

rename classes in pcse_test_code to avoid pytest warning

5da8c52

add module docstring

70b9827

remove custom_dir from mkdocs for now

e7ac574

fix linter errors

5f88f43

SarahAlidoost mentioned this pull request Aug 20, 2025

Explore "Hybrid Recurrent Models for Accurate Crop State Predictions" including WOFOST #13

Closed

add more tests for gradients

576ece0

cwmeijer mentioned this pull request Sep 1, 2025

Remove deprecated linting rules from pyproject.toml.jinja NLeSC/python-template#690

Merged

13 tasks

refactor: extract model creation method

2ad2d3e

cwmeijer approved these changes Sep 1, 2025

View reviewed changes

Comment thread tests/physical_models/crop/test_leaf_dynamics.py Outdated

Comment thread tests/physical_models/crop/test_leaf_dynamics.py

Comment thread src/diffwofost/physical_models/crop/leaf_dynamics.py

SarahAlidoost and others added 2 commits September 1, 2025 14:12

Merge pull request #15 from WUR-AI/leaf_dynamics_test_refactor

6580640

refactor: extract model creation method

rename variables making them readable, as suggested in review

053d868

SarahAlidoost marked this pull request as ready for review September 1, 2025 12:27

SarahAlidoost added 2 commits September 1, 2025 14:44

change torch dtype to float64

64f36f1

add two tests for calculating gradients numerically

5ecb89c

SarahAlidoost mentioned this pull request Sep 1, 2025

[Backlog Item]: Build a prototype of differentiable wofost72_pp #3

Open

fix ruff formatting issues

0984aa5

SarahAlidoost added this to DeltaCrop: epic1 and epic2 Sep 1, 2025

SarahAlidoost moved this to In review in DeltaCrop: epic1 and epic2 Sep 1, 2025

This was referenced Sep 2, 2025

[Tasks]: Make the computations of leaf_dynamic vectorized #19

Closed

[Task] running individual pcse module with TestEngine #22

Closed

cwmeijer reviewed Sep 2, 2025

View reviewed changes

Comment thread tests/physical_models/crop/test_leaf_dynamics.py

SarahAlidoost requested a review from michielkallenberg September 3, 2025 07:15

SarahAlidoost added 3 commits September 3, 2025 11:38

fix assert message

3d62464

dont use detach before clone

b84b16e

fix tests for numerical gradients

468d89c

This was referenced Sep 4, 2025

how to validate the results after introducing approximations #24

Open

[Tasks]: Implement Data array organization inside Engine #25

Open

SarahAlidoost mentioned this pull request Sep 16, 2025

[Task]: use all test data for unit tests #27

Closed

SarahAlidoost added 3 commits September 16, 2025 15:19

fix initial values of rates, and apply mask when DVS is negative

475ada8

fix ruff formatting

9753a52

make ruff happy

5b98a3e

SarahAlidoost mentioned this pull request Sep 16, 2025

Make the Root dynamics differentiable #29

Merged

fix with patch block

35a9ed0

michielkallenberg reviewed Sep 17, 2025

View reviewed changes

michielkallenberg approved these changes Sep 17, 2025

View reviewed changes

SarahAlidoost added 2 commits September 18, 2025 09:58

fix outlining in doc string

401b3d8

complete SPAN TWLV test

309369c

SarahAlidoost merged commit d8a5a56 into main Sep 18, 2025
8 of 10 checks passed

github-project-automation Bot moved this from In review to Done in DeltaCrop: epic1 and epic2 Sep 18, 2025

SarahAlidoost deleted the leaf_dynamics branch September 18, 2025 13:41

SarahAlidoost mentioned this pull request Sep 29, 2025

[Tasks]: Make two modules of wofost72_pp differentiable #8

Closed

18 tasks

		assert_almost_equal(numerical_grad, grads.item(), decimal=3)


		class TestDiffLeafDynamicsSPAN:

		self.TERMINAL_OUTPUT_VARS = []


		class EngineTestHelper(Engine):

Conversation

SarahAlidoost commented Aug 6, 2025

Uh oh!

cwmeijer commented Aug 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cwmeijer left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SarahAlidoost commented Sep 1, 2025

Uh oh!

SarahAlidoost commented Sep 1, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michielkallenberg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SarahAlidoost Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SarahAlidoost commented Sep 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cwmeijer commented Aug 28, 2025 •

edited

Loading

cwmeijer left a comment •

edited

Loading

SarahAlidoost Sep 18, 2025 •

edited

Loading