-
Notifications
You must be signed in to change notification settings - Fork 106
Explicit validator for IMAS core_profiles IDS. #1866
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
base: main
Are you sure you want to change the base?
Conversation
56d492c to
76ac7b6
Compare
c7bfcbe to
ae47fed
Compare
PiperOrigin-RevId: 846714287
ae47fed to
9f4d52d
Compare
MateoBell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And just a general comment do you think it would be relevant to have a flag to raise error if any value is missing ? We talked about it before but I think its okay without.
|
|
||
|
|
||
| def _validate_profile_conditions(ids: ids_toplevel.IDSToplevel) -> None: | ||
| """Validates the profiles_conditions IDS.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: validates core_profiles IDS for profiles_conditions
|
|
||
| def _validate_profile_conditions(ids: ids_toplevel.IDSToplevel) -> None: | ||
| """Validates the profiles_conditions IDS.""" | ||
| for profiles_1d in ids.profiles_1d: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If any quantity, e.g density is missing in each profiles_1d (i.e at each time slice), it might get a bit verbose as it will give a warning for every time slice ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point. Would it be sufficient to only check the first slice? Could we assume the rest have the same structure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for now as we only check if it is filled, I think so. If we go further to check positivity of temp and densities or other things we might need to check all time slices.
Explicit validator for IMAS core_profiles IDS.