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

Convert to data frame with Rcpp #857

Merged
merged 6 commits into from Aug 5, 2021
Merged

Convert to data frame with Rcpp #857

merged 6 commits into from Aug 5, 2021

Conversation

kylebaron
Copy link
Collaborator

@kylebaron kylebaron commented Jul 30, 2021

Summary

This pr implements as.data.frame.matrix in Rcpp so that we can pass back a data frame from the simulation code. The changes in this PR are not user-facing.

The implementation follows base::as.data.frame.matrix().

src/mrgsolve.cpp Outdated
@@ -388,6 +388,19 @@ Rcpp::List EXPAND_OBSERVATIONS(
Rcpp::Named("index") = index);
}

Rcpp::List mat2df(Rcpp::NumericMatrix& x) {

Choose a reason for hiding this comment

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

I'm not familiar with Rcpp df API. Could you walk me through this function? Thanks. @kylebaron

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The pattern basically follows base::as.data.frame.matrix()

  1. Pass in a matrix
  2. create a list with the same number of slots as the matrix has columns
  3. Iterate through the matrix columns and copy into the list
  4. Rcpp::_ is just a way to get all the rows (or all columns)
  5. The rn(2) bit replicates what happens in base::.set_row_names
  6. Set the class to data frame
  7. Add row names
  8. Return

as.data.frame.matrix() does some other stuff but this does the essential steps for what I need.

Choose a reason for hiding this comment

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

So mat2df doesn't take ownership of the matrix x's data. Perhaps use const to ensure x is not modified? Like

Rcpp::List mat2df(Rcpp::NumericMatrix const& x) {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure.

Copy link

@yizhang-yiz yizhang-yiz 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 to me!

@kylebaron
Copy link
Collaborator Author

Thanks @yizhang-yiz

@kylebaron kylebaron merged commit 3343eb5 into develop Aug 5, 2021
@kylebaron kylebaron deleted the refactor/mat2df branch August 5, 2021 04:22
@kylebaron kylebaron modified the milestone: v0.11.2 Aug 29, 2021
@kylebaron kylebaron mentioned this pull request Aug 31, 2021
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.

None yet

2 participants