[Cross Validation] Added cross validation rules for the SC module#2916
[Cross Validation] Added cross validation rules for the SC module#2916matthew7838 wants to merge 43 commits intodevfrom
Conversation
|
Current Coverage: 99% Mypy errors on crop-field-xvalidation branch: 1195 |
|
🚨 Please update the changelog. This PR cannot be merged until |
github-actions
bot
force-pushed
the
crop-field-xvalidation
branch
from
3d4371d to
040785d
Compare
github-actions
bot
force-pushed
the
crop-field-xvalidation
branch
from
040785d to
c8ecd97
Compare
|
Current Coverage: % Mypy errors on crop-field-xvalidation branch: 1205 |
|
🚨 Please update the changelog. This PR cannot be merged until |
|
Current Coverage: % Mypy errors on crop-field-xvalidation branch: 1205 |
|
🚨 Please update the changelog. This PR cannot be merged until |
…p-field-xvalidation
…p-field-xvalidation
|
Current Coverage: % Mypy errors on crop-field-xvalidation branch: 1205 |
|
🚨 Please update the changelog. This PR cannot be merged until |
allisterakun
left a comment
allisterakun
left a comment
There was a problem hiding this comment.
Great job, Matthew! All these rules are valid, but I have just added some suggestions for improvements.
|
Current Coverage: 99% Mypy errors on crop-field-xvalidation branch: 1191 |
|
🚨 Unauthorized changes detected in protected files. Please remove these changes if they are not intended. |
|
Current Coverage: % Mypy errors on crop-field-xvalidation branch: 1191 |
|
🚨 Unauthorized changes detected in protected files. Please remove these changes if they are not intended. |
|
Current Coverage: 99% Mypy errors on crop-field-xvalidation branch: 1191 |
|
🚨 Unauthorized changes detected in protected files. Please remove these changes if they are not intended. |
| "description": "alfalfa_silage temperature and harvest index bounds", | ||
| "target_and_save": { | ||
| "variables": { | ||
| "alfalfa_silage_minimum_temperature": "crop_configurations.crop_configurations.0.minimum_temperature", |
There was a problem hiding this comment.
We do something like this with at least one of the animal-related cv rules and I think basing a rule on the specific index of an item within a list seems really unreliable and problematic and hard to accommodate for new users setting up their own crop config files or even tweaking the example one. I think #2700 could hopefully help with something like this because really this rule is specific to alfalfa silage, not crop config item 0.
There was a problem hiding this comment.
Do you think we should merge this first and come back to fix it when #2700 is done? Or should we wait for it?
There was a problem hiding this comment.
Maybe we can discuss at WT today
There was a problem hiding this comment.
I actually think the way this should be done is to refactor the input file (and of course then the way the input file is processed/read by both IM and in the C&S module) to be a dict of dicts with crop_name as the key:
e.g.
{
"crop_configurations": {
"alfalfa_silage": {
"plant_category": "perennial_legume",
"is_nitrogen_fixer": true,
...
},
"alfalfa_baleage": {
"plant_category": "perennial_legume",
"is_nitrogen_fixer": true,
...
}
}
}This would prevent accidental duplication and make these rules more concrete. We can discuss at WT but just wanted to share my initial thoughts. This would likely need to be a separate issue since it would involve changes in the metadata and in how this data is processed in C&S module.
github-actions
bot
force-pushed
the
crop-field-xvalidation
branch
from
d69ba2b to
9a99a5a
Compare
github-actions
bot
force-pushed
the
crop-field-xvalidation
branch
from
9a99a5a to
516e03a
Compare
|
Current Coverage: 99% Mypy errors on crop-field-xvalidation branch: 1191 |
|
🚨 Unauthorized changes detected in protected files. Please remove these changes if they are not intended. |
4 participants
Added cross-validation rules for the Crop and Soil module.
Context
Issue(s) closed by this pull request: closes #2885 #2886
What
crop_and_soil_cross_validation.jsonandfield_cross_validation.json.DataValidatorcalled_evaluate_equal_data_lengthto check the length of list-type inputs.dev.Why
We currently lacks module specific cross validation rules.
How
Added the rules and the function that are general enough.
Test plan
Input Changes
Output Changes
Filter