1293 update secir groups surrogate models#1297
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1297 +/- ##
==========================================
- Coverage 97.28% 97.27% -0.01%
==========================================
Files 168 171 +3
Lines 14858 15117 +259
==========================================
+ Hits 14454 14705 +251
- Misses 404 412 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…_groups/generate_dampings.py replaced by dampings.py
Inserting current main branch
HenrZu
left a comment
There was a problem hiding this comment.
Looks very good :) Thank you!
Some comments. My main remark is that we should check the secir simple code again and outsource doubled code into an model independent directory/file (e.g. "utils").
Functions should only be stored in the correpsonding model files when they are strongly model dependent and not usable other otherwise :)
regarding the failing ci, you can just print the path, where the data is saved to. Then you see the correct one in the ci log.
HenrZu
left a comment
There was a problem hiding this comment.
Thank you for the changes :) It would be great if you could create a new test file that tests the utils. These should be model-independent. And therefore do not have to be tested within the model tests
HenrZu
left a comment
There was a problem hiding this comment.
only 1 minimal change left. Great work :)
…_groups/README.md Co-authored-by: Henrik Zunker <[email protected]>
Changes and Information
Please briefly list the changes (main added features, changed items, or corrected bugs) made:
If need be, add additional information and what the reviewer should look out for in particular:
Merge Request - Guideline Checklist
Please check our git workflow. Use the draft feature if the Pull Request is not yet ready to review.
Checks by code author
Checks by code reviewer(s)