Skip to content

Finalizing the hybrid prototype#119

Open
SarahAlidoost wants to merge 24 commits into
mainfrom
fix_111
Open

Finalizing the hybrid prototype#119
SarahAlidoost wants to merge 24 commits into
mainfrom
fix_111

Conversation

@SarahAlidoost
Copy link
Copy Markdown
Collaborator

@SarahAlidoost SarahAlidoost commented May 11, 2026

Closes #111

with this PR:

  • some validation and checks are done using config when configuration is created or in init of Engine, so the wofost initialize method does only the initialization of the components, as before.
  • normalization of crop_components is done in init of Engine
  • there is now an extra config argument CROP_NN_MODEL, this way an individual crop ml_model can be run using engine, see tests for details.
  • the nb is ran again, updating the path to the weather excel file, since the file is fixed in pcse/notebooks repo

@SarahAlidoost SarahAlidoost requested a review from SCiarella May 12, 2026 13:05
@SarahAlidoost SarahAlidoost marked this pull request as ready for review May 13, 2026 07:26
Copy link
Copy Markdown
Collaborator

@SCiarella SCiarella left a comment

Choose a reason for hiding this comment

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

Well done @SarahAlidoost 👍
The introduction of simulationobject.py and ovveride.py looks much cleaner.

I think we need to explain somewhere in the docs or even in the hybrid notebook how to use CROP_NN_MODEL because right now it would be hidden from the user.

Comment thread src/diffwofost/physical_models/base/simulationobject.py Outdated
@SarahAlidoost
Copy link
Copy Markdown
Collaborator Author

Well done @SarahAlidoost 👍 The introduction of simulationobject.py and ovveride.py looks much cleaner.

Thanks for the suggestions.

I think we need to explain somewhere in the docs or even in the hybrid notebook how to use CROP_NN_MODEL because right now it would be hidden from the user.

Right! I won't add it to the same hybrid notebook because it makes it very busy. But I noticed that our doc is not updated regarding ml_models. I added it as a task to issue #107.

@SarahAlidoost SarahAlidoost requested a review from SCiarella May 18, 2026 09:08
Copy link
Copy Markdown
Collaborator

@SCiarella SCiarella left a comment

Choose a reason for hiding this comment

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

@SarahAlidoost perfect! ready to merge

@sonarqubecloud
Copy link
Copy Markdown

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.

[Task] Finalizing the hybrid prototype

2 participants