Skip to content

Make 'partitioning' differentiable/vectorized#70

Merged
SCiarella merged 25 commits into
mainfrom
partitioning
Jan 14, 2026
Merged

Make 'partitioning' differentiable/vectorized#70
SCiarella merged 25 commits into
mainfrom
partitioning

Conversation

@SCiarella
Copy link
Copy Markdown
Collaborator

@SCiarella SCiarella commented Jan 6, 2026

Target issue #41.

In implementing the partitioning module, I have realized that the x-values of Afgen tables are actually not fully differentiable (see test_x_breakpoint_at_clamp in test_utils.py). I belive that in a real scenario, the user will be interested in optimizing the y-values but not the x-values, so I am just ignoring them in test_partitioning.

More than once during the tests the _check_partitioning warning gets triggered, suggesting that the sum of the partitions is not 1. To me this behavior looks suspicious, however the tests produce the expected results.

@SCiarella SCiarella changed the title Attempt to make differentiable Partitioning differentiable/vectorized Jan 6, 2026
@SCiarella SCiarella changed the title Partitioning differentiable/vectorized Make 'partitioning' differentiable/vectorized Jan 7, 2026
Base automatically changed from gpu to main January 8, 2026 15:39
@SCiarella SCiarella marked this pull request as ready for review January 9, 2026 13:28
Comment thread src/diffwofost/physical_models/crop/partitioning.py
Comment thread src/diffwofost/physical_models/crop/partitioning.py Outdated
Comment thread src/diffwofost/physical_models/crop/partitioning.py
Comment thread src/diffwofost/physical_models/crop/partitioning.py Outdated
Comment thread src/diffwofost/physical_models/crop/partitioning.py
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 the implementation 👍 . It looks good. Just minor suggestions for docstring.

Regarding your comments on _check_partitioning, I found out the warning is triggered only in the class TestDiffPartitioningGradients and not TestPartitioning. The reason might be the random test values for different parameters. To me this is fine.

Also, another thing to check later is that if the parameters of the Partitioning can be optimized in general (to be discussed in the update meeting?!). If so, we can add a notebook to show the optimization, similar to other modules.

SCiarella and others added 4 commits January 14, 2026 09:16
Co-authored-by: SarahAlidoost <55081872+SarahAlidoost@users.noreply.github.com>
Co-authored-by: SarahAlidoost <55081872+SarahAlidoost@users.noreply.github.com>
Co-authored-by: SarahAlidoost <55081872+SarahAlidoost@users.noreply.github.com>
@sonarqubecloud
Copy link
Copy Markdown

@SCiarella SCiarella merged commit 7a447f2 into main Jan 14, 2026
11 checks passed
@SCiarella SCiarella deleted the partitioning branch January 14, 2026 08:48
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