Skip to content

Add methods for piecewise exponential distributions#137

Merged
dincerti merged 12 commits intohesim-dev:mainfrom
mclements:piecewise_exponential
Jun 25, 2025
Merged

Add methods for piecewise exponential distributions#137
dincerti merged 12 commits intohesim-dev:mainfrom
mclements:piecewise_exponential

Conversation

@mclements
Copy link
Copy Markdown
Collaborator

Devin,

As a start, here are some modest changes for piecewise exponential distributions.

Please shout out if anything is unclear.

Kindly, Mark.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 25, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.72%. Comparing base (e218e24) to head (2523781).
Report is 35 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #137      +/-   ##
==========================================
+ Coverage   92.51%   92.72%   +0.20%     
==========================================
  Files          64       64              
  Lines        5544     6252     +708     
==========================================
+ Hits         5129     5797     +668     
- Misses        415      455      +40     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dincerti dincerti requested a review from Copilot May 3, 2025 15:41
@dincerti dincerti self-requested a review as a code owner May 3, 2025 15:41
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for piecewise exponential distributions by introducing new distribution methods and their corresponding Rcpp exports, along with minor utility functions to support these implementations.

  • Added Rcpp-exported test functions (hesim_bound and member_of) in test-utils.cpp and RcppExports.cpp.
  • Extended the Rcpp module in distributions.cpp to include new methods (pdf, cdf, quantile, hazard, cumhazard) for the piecewise_exponential class.
  • Updated the distributions header to compute the cumulative rate parameters and define new methods for the piecewise exponential distribution.

Reviewed Changes

Copilot reviewed 6 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/test-utils.cpp Added test functions for hesim_bound and member_of.
src/distributions.cpp Registered new piecewise exponential methods via the Rcpp module.
src/RcppExports.cpp Exposed new C_test_hesim_bound and C_test_member_of functions.
inst/include/hesim/utils.h Added utility functions hesim_bound and member_of with new includes.
inst/include/hesim/stats/distributions.h Updated piecewise_exponential with cumulative rate computations and added distribution methods.
Files not reviewed (3)
  • R/RcppExports.R: Language not supported
  • tests/testthat/test-cpp-distributions.R: Language not supported
  • tests/testthat/test-cpp-utils.R: Language not supported
Comments suppressed due to low confidence (2)

inst/include/hesim/utils.h:304

  • Consider passing 'range' as a const reference (i.e., const std::vector& range) to avoid unnecessary copying.
inline int hesim_bound(double x, std::vector<double> range) {

inst/include/hesim/utils.h:315

  • Consider passing 'lookup' as a const reference (i.e., const V& lookup) to avoid unnecessary copying.
template<class T, class V>
  bool member_of(T x, V lookup) {

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Member

@dincerti dincerti left a comment

Choose a reason for hiding this comment

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

Looks good! Just some minor comments on function names and the edge case.

@mclements
Copy link
Copy Markdown
Collaborator Author

Devin: I have also added some casts in a number of integer comparisons to address some long-standing warnings.

@dincerti dincerti merged commit 4cab283 into hesim-dev:main Jun 25, 2025
7 checks passed
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.

4 participants