Validate inputs when reading them in #8
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
confunguido
requested changes
Nov 23, 2024
src/parameters_loader.rs
Outdated
| // in the long term that makes sense for an arbitrary GI | ||
| if parameters.generation_interval <= 0.0 { | ||
| return Err(IxaError::IxaError( | ||
| "r_0 must be a non-negative number".to_string(), |
There was a problem hiding this comment.
It sounds like we don't know yet how to specify generation interval. Should we remove this for now and only leave r_0? Or at least change the error so that it doesn't say r_0
There was a problem hiding this comment.
Oops good catch, thanks! I like having this code there as a template for now and then we can change it as we decide on an input schema for the GI if you are ok with that?
src/parameters_loader.rs
Outdated
| #[test] | ||
| fn test_validate_r_0() { | ||
| let parameters = ParametersValues { | ||
| population: 1000, |
There was a problem hiding this comment.
We are not using population
There was a problem hiding this comment.
Yes, agree -- I requested this to be removed in the population loader PR as that makes the most thematic sense, so I think we need to land that first. I will rebase this to main once we land that.
ChiragKumar9
force-pushed
the
ckk_validate_inputs
branch
from
1c2895f to
045ea20
Compare
ChiragKumar9
force-pushed
the
ckk_validate_inputs
branch
2 times, most recently
from
ba2ebb9 to
74a4ce0
Compare
ChiragKumar9
force-pushed
the
ckk_validate_inputs
branch
from
74a4ce0 to
cc4f1e4
Compare
confunguido
approved these changes
Nov 27, 2024
KOVALW
added a commit
that referenced
this pull request
Dec 4, 2024
* Added population loader * Fix header and added example synth pop to read * Fixed homeId camel case issue with precommit and added error propagation * First test for pop loader * Precommit fmt changes * Census guardrail first pass * Second test of invalid census tract * loading population as bytes * Add test to synth population loader * pre-commit fixes * Changed name of test * Pre-commit fixes * Comment changes and small fmt changes * Pre-commit change * Removed population from input json * Fixed tests with helper fxn and proper results * person_id temporary made * Move clone so you're cloning less * Import ixa error to parameters and fmt change * Added IxaError propagation for parse * Expected err message for test 2 * Fix header and added example synth pop to read * Report skeleton * Updating parameters for report * Periodic report using add_periodic_plan_with_phase * Remove duplicated csv dependency * Fix index property * input file with periodic report * Validate inputs when reading them in (#8) * validation fcn * review comments * pre-commit fixes * new ixa syntax for validator in define_global_properties * playing withpre-commit * updated pathbuf syntax * explicit error messages in tests * add population_periodic_report to parameters values in test * Migrated index property to main * Add command line argument to overwrite reports (#20) * flag for overwriting output files * use consistent formatting of default value * Tests: pop size and fixed unwrap * Finished error propagation * Last propagation in byte_headers * Removed excess use and moved mod's * Fixed test to cehck home id * Migrated to use add_periodic_report * update tests to explicitly check the error types * Added write context arguments * Deleted original periodic report script * Added force_overwrite to report config * Replace call to get_person_id * FMT --------- Co-authored-by: Guido Espana <[email protected]> Co-authored-by: Guido Espana <[email protected]> Co-authored-by: EKR <[email protected]> Co-authored-by: Chirag Kumar <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Provides a skeleton of checking whether inputs are valid based on our knowledge of them prior to them being used in the model in case they will not be used till later in the model running to try to catch errors early.