Skip to content
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

677-computation-of-the-effective-reproduction-number-in-the-secir-model #698

Conversation

johapau
Copy link
Collaborator

@johapau johapau commented Jul 13, 2023

Changes

Please briefly list the changes made:

[Describe here in 2-3 lines. For details, reference to issues to avoid redundant information.]

Merge Request - GuideLine Checklist

  • Check our git workflow before opening a Pull request/Merge request.
  • Request a reviewer when your work is ready to review, before this please use the draft feature.

Checks by code author

  • Every addressed issue is linked (use the "Closes #ISSUE" keyword below)
  • New code adheres to coding guidelines
  • No large data files have been added (files should in sum not exceed 100 KB, avoid PDFs, Word docs, etc.)
  • Tests are added for new functionality and a local test run was successful
  • Appropriate documentation for new functionality has been added (Doxygen in the code and Markdown files if necessary)
  • Proper attention to licenses, especially no new third-party software with conflicting license has been added

Checks by code reviewer(s)

  • Corresponding issue(s) is/are linked and addressed
  • Code is clean of development artifacts (no deactivated or commented code lines, no debugging printouts, etc.)
  • Appropriate unit tests have been added, CI passes and code coverage is acceptable (did not decrease)
  • No large data files added in the whole history of commits(files should in sum not exceed 100 KB, avoid PDFs, Word docs, etc.)

Additional context

Please list additional information or things a reviewer should look out for.

@johapau johapau linked an issue Jul 13, 2023 that may be closed by this pull request
2 tasks
@johapau johapau self-assigned this Jul 13, 2023
@johapau johapau changed the title Added reproduction number files to the branch 677-computation-of-the-effective-reproduction-number-in-the-secir-model Jul 13, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jul 13, 2023

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (0904a39) 94.22% compared to head (b0b451c) 95.01%.
Report is 33 commits behind head on main.

❗ Current head b0b451c differs from pull request most recent head 200c9b7. Consider uploading reports for the commit 200c9b7 to get more accurate results

Files Patch % Lines
cpp/models/ode_secir/model.h 94.73% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #698      +/-   ##
==========================================
+ Coverage   94.22%   95.01%   +0.78%     
==========================================
  Files         108      112       +4     
  Lines        8540     8928     +388     
==========================================
+ Hits         8047     8483     +436     
+ Misses        493      445      -48     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@johapau johapau requested review from HenrZu and mknaranja September 8, 2023 11:28
*@returns The computed reproduction number at the provided index time.
*/
template <class Base>
IOResult<ScalarType> get_reproduction_number(size_t t_idx, const Simulation<Base>& sim)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know/assume that this contains the derivative of the ODE so a lot of code-doubling happens. Generally it is just not a very good a idea to introduce redudant code parts. We should maybe think of how only one responsible code part returns for instance the mechanism of X and then this is used for get_derivatives and get_reproduction_number.

Copy link
Contributor

@HenrZu HenrZu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your work. Looks good. I made a few comments. We should also consider making the code more modular. In general we should avoid duplication of code. And increased modularity also creates a better opportunity for testing functionalities.

Im currently checking the equations separately and also try to verify the tested values.

cpp/models/ode_secir/model.h Outdated Show resolved Hide resolved
cpp/models/ode_secir/model.h Outdated Show resolved Hide resolved
cpp/models/ode_secir/model.h Outdated Show resolved Hide resolved
cpp/models/ode_secir/model.h Outdated Show resolved Hide resolved
cpp/models/ode_secir/model.h Outdated Show resolved Hide resolved
cpp/models/ode_secir/model.h Outdated Show resolved Hide resolved
@johapau johapau closed this Dec 4, 2023
@johapau johapau deleted the 677-computation-of-the-effective-reproduction-number-in-the-secir-model branch December 4, 2023 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Computation of the Effective Reproduction Number in the SECIR Model
4 participants