Modularization and rewrite of the main functionality#69
Conversation
…it is different to 1. Might reconsider adding it in the future
…already computed as `age_head`.
jhoshiko
left a comment
jhoshiko
left a comment
There was a problem hiding this comment.
Nice! The modularization changes look fantastic and while there are plenty of issues remaining to be addressed (unrelated to modularization), I think this all looks good aside for a few comments I've left (mostly clarification questions).
Before I approve this pull request, I would like to run the refactored code on my machine too (this won't catch any small issues but is quick check to see if anything explodes on my end). Are the installation instructions good enough to test the build on Windows?
Sounds good! I'll go through the comments in the meantime. It should be fine to create the environment on windows with the |
Fix small logic errors found in original code
|
@yamilbknsu I've install and ran this branch and everything is working on my end! I think we can consider this PR finished and ready to merge! |
The
model.pyfile which originally contained all the logic for all of the modules was broken up into individual files in a newmodelsmodule. The logic of each function was refactored for consistency, readability and scalability.Several behaviors were changed according to discussions with the group, which were documented in an internal file. After the refactoring, a benchmark was performed to make sure any differences in simulation output were small enough to be attributed to randomness or minor edge cases.
While some of the structure of the modules is going to change in the future to accommodate further enhancements (refactoring of the calibration code and cleaning of input data for instance), this version of the repo provides improved speeds that can be leveraged in parallel efforts.
With this PR, I aim to:
orcafunctionscloses #62