Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

9694 Renamed all Nconc to NConc #9695

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

par456
Copy link
Collaborator

@par456 par456 commented Feb 19, 2025

Resolves #9694

Need to see if this effects stats or if it break validation.

@par456
Copy link
Collaborator Author

par456 commented Feb 19, 2025

Stats are the same, just renamed to the new correct names. Barley, Mungbean and Soybean also have additional stats that are now linking thanks to the names being correct.

@BrianCollinss
Copy link
Member

If you don't mind, I put these here as I see lots of suspicious zeros (some look like wrong units been used).

image

image

image

image

image

image

@par456
Copy link
Collaborator Author

par456 commented Feb 19, 2025

Thanks Brian, yeah not going to merge this in until I can get these new POStats cleaned up somewhat. I assume when they were added to observed files they weren't cleaned since they weren't appearing, but want to double check that first.

@par456
Copy link
Collaborator Author

par456 commented Feb 21, 2025

I've gone and had a look at the data for those problem points, some I fixed, others I left for when the model is worked on next. Description below for each.

Mungbean

  • Removed 0 from Pod.NConc
  • The other strange point does calculate correctly from N and biomass, so there may be an entry error here. Not fixing in this PR, but should be addressed with a cleanup later.

Oats

  • Leaf.Dead.NConc ranges from 0 to 1.7, which suggests it is in the wrong units. Not fixing here however as its an existing PO stats.

Soybean.Pod.NConc

  • Points at 0 on the Predicted are due to APSIM having not developed pods by the matching date (observation on the 10th, apsim pods appear on the 11th). These points are correct, and the model is not matching observed.
  • Ames points seem to have the wrong units, but that's a particular dataset that only the modeller should touch.

Wheat.AboveGround.NConc

  • Fixed unit error in FAR observed data

@par456 par456 requested a review from hol353 February 21, 2025 07:09
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.

Inconsistent naming of NConc Variables
2 participants