Skip to content

Waterlogging models#10503

Open
ilhuber wants to merge 66 commits intoAPSIMInitiative:masterfrom
ilhuber:10502WLModels
Open

Waterlogging models#10503
ilhuber wants to merge 66 commits intoAPSIMInitiative:masterfrom
ilhuber:10502WLModels

Conversation

@ilhuber
Copy link
Copy Markdown
Contributor

@ilhuber ilhuber commented Sep 22, 2025

Resolves #10502

This will hopefully show model improvement. If we see differences in non waterlogging simulations we know there is some error. There may be some hidden cultivars in some of the private datasets.

This will make it easier to track specific model diffs without
accumulated changes to [JsonIgnore] type fields.
@ilhuber
Copy link
Copy Markdown
Contributor Author

ilhuber commented Sep 22, 2025

Looks like a canola change slipped through somehow. I think it might be phase definitions in Leaf senescence rate.

@ilhuber ilhuber added Do Not Merge Used for PRs that should not be merged for any reason RP Follow-up labels Sep 22, 2025
@par456 par456 added Ready for Reference Panel Review Used for PRs that should be added to the agenda for APSIM Reference Panel meetings. and removed RP Follow-up labels Sep 22, 2025
The CoverGreen variable was a proxy for a different value APSIM does not
predict during calibration.
These were not part of the prior model, nor are we aiming to add them.
@hut104
Copy link
Copy Markdown
Contributor

hut104 commented Sep 29, 2025

The Canola model seems to have the Water logging functions parametersed out (ie to have no effect). The water logging factors are only turned on for individual cultivars.
I suggest that the factors need to be turned on in the plant base model.

It is not clear as to why the converter is adding defaults into cultivars. That is the purpose of the base model.

@hut104
Copy link
Copy Markdown
Contributor

hut104 commented Sep 29, 2025

Is it possible to explain what the purpose of the converter is? This seems to be updating parameters in some cultivars.

@ilhuber
Copy link
Copy Markdown
Contributor Author

ilhuber commented Sep 29, 2025

The purpose of the converter is to update a cultivar path that had targeted a location in the model tree which was moved deeper due to the waterlogging updates. It "re-targets" these paths. Looking at the diffs, apparently the only model that had default cultivars get re-targeted was Maize. Still, the converter would be necessary to preserve user's custom cultivars.

Not sure where you are seeing the converter altering defaults? Those later commits are not related to the converter. Defaults in the Canola model had been altered by whoever did the work on the Canola replacement model and it was having an effect on the non-waterlogging simulations.

ilhuber added 2 commits March 3, 2026 10:06
This got put in a wrong spot in the previous commit. I was wondering why
there were no merge conflicts...
@ilhuber
Copy link
Copy Markdown
Contributor Author

ilhuber commented Mar 3, 2026

@par456 @ric394 @hut104

What is the new way of retest-this-please-jenkinsing for errors like the above?

@par456
Copy link
Copy Markdown
Collaborator

par456 commented Mar 3, 2026

At the moment, if its one of the github actions. Click on the one that has failed, then in the top right of the screen, click re-run all jobs

image

@ilhuber ilhuber mentioned this pull request Mar 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Do Not Merge Used for PRs that should not be merged for any reason Ready for Reference Panel Review Used for PRs that should be added to the agenda for APSIM Reference Panel meetings.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Waterlogging models

3 participants