Skip to content

Make leaf differentiable and vectorized#56

Merged
SCiarella merged 13 commits into
mainfrom
leaf_diff
Nov 20, 2025
Merged

Make leaf differentiable and vectorized#56
SCiarella merged 13 commits into
mainfrom
leaf_diff

Conversation

@SCiarella
Copy link
Copy Markdown
Collaborator

This PR addresses #45.

It follows the test structure of #55 to ensure that leaf parameters are differentiable and vectorizable.
Before merging, the issues with the vectorization of drv should be addressed.

@SCiarella
Copy link
Copy Markdown
Collaborator Author

SCiarella commented Nov 18, 2025

In order to work with vectorized drv data, this PR implements _ensure_shape_finalized which is called inside calc_rates. The function has the role of reshaping the parameters if multiple temperatures are provided. Similarly to all the other parameters, we first ensure consistency between vectorized shapes, so all the parameters + drv.TEMP can only be scalar or the same vector shape.

This PR also implements the function utils._check_drv_shape that ensures consistency within every drv instance. Once the weather module is updated and vector shape checks are included in the main class, this function can be removed.

Copy link
Copy Markdown
Collaborator

@SarahAlidoost SarahAlidoost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SCiarella thanks for implementation and tests 👍 . Please have a look at my comments. The main issue with the current implementation is that the physical models shouldn’t be worry about the shape or format of their inputs. At this stage, we assume the physical model receives correctly shaped data, so it can do vectorized computations and proper gradient tracking. Everything else should be handled by the Engine. The physical model can include basic checks, but it shouldn’t go more than that.

If anything in my comments is unclear, please let me know, we can discuss the details.

Comment thread src/diffwofost/physical_models/crop/leaf_dynamics.py Outdated
Comment thread src/diffwofost/physical_models/crop/leaf_dynamics.py Outdated
Comment thread src/diffwofost/physical_models/crop/leaf_dynamics.py
Comment thread src/diffwofost/physical_models/crop/leaf_dynamics.py Outdated
Comment thread src/diffwofost/physical_models/crop/leaf_dynamics.py Outdated
Comment thread src/diffwofost/physical_models/crop/leaf_dynamics.py Outdated
Comment thread src/diffwofost/physical_models/utils.py
Comment thread src/diffwofost/physical_models/utils.py Outdated
Comment thread src/diffwofost/physical_models/utils.py
Comment thread src/diffwofost/physical_models/crop/leaf_dynamics.py
Comment thread src/diffwofost/physical_models/crop/leaf_dynamics.py Outdated
Copy link
Copy Markdown
Collaborator

@SarahAlidoost SarahAlidoost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SCiarella looks great! thanks for addressing my comments. Just one test needed for weather data as 2d array, as we discussed :)

@sonarqubecloud
Copy link
Copy Markdown

@SCiarella SCiarella merged commit bf1621f into main Nov 20, 2025
11 checks passed
@SCiarella SCiarella deleted the leaf_diff branch November 20, 2025 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants