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.

Sign up for GitHub

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

Jump to bottom

Use fromRVector() and toRVector() in ParameterVector and RealVector #766

Open
Bai-Li-NOAA opened this issue Mar 18, 2025 · 0 comments
Open

Use fromRVector() and toRVector() in ParameterVector and RealVector #766

Bai-Li-NOAA opened this issue Mar 18, 2025 · 0 comments
Labels
status: triage_needed This is not approved for this milestone, do not work on it yet

Comments

@Bai-Li-NOAA
Copy link
Contributor

The interface refactor PR introduces fromRVector() and toRVector() methods for RealVector. These methods simplify usage by being more concise than the previous approach, which required calling resize(), purrr::walk(), and set() to assign values. It would be beneficial to implement these methods consistently in ParameterVector as well. Once that's done, we can update the tests to use fromRVector().

Originally posted by @Bai-Li-NOAA in #765 (comment)
@kellijohnson-NOAA kellijohnson-NOAA added the status: triage_needed This is not approved for this milestone, do not work on it yet label Mar 18, 2025
kellijohnson-NOAA added a commit that referenced this issue Mar 18, 2025
@kellijohnson-NOAA
refactor(rcpp): Refactor rcpp interface
a938844 
Keep all members in a shared pointer allowing copies to be made when an
object goes out of scope. These changes are needed to allow finalize()
to work and not crash the R session. RealVector was created as a part
of this work. ParameterVector are now set using resize() and purrr::walk()
rather than new().

TODO:
* Some additional thought shouldd go into using fromRVector and toRVector
  in future versions of this code. See #766
* Update zzz.R to set methods for RealVector that handle indexing
  translation (e.g., R uses 1-based indexing, C++ uses 0-based indexing).
* Modify the relevant R code that assigns values to RealVector to
  accommodate the updated indexing.
@kellijohnson-NOAA kellijohnson-NOAA mentioned this issue Mar 18, 2025
Merged
kellijohnson-NOAA added a commit that referenced this issue Mar 18, 2025
@kellijohnson-NOAA
refactor(rcpp): Refactor rcpp interface
7fc2dd6 
Keep all members in a shared pointer allowing copies to be made when an
object goes out of scope. These changes are needed to allow finalize()
to work and not crash the R session. RealVector was created as a part
of this work. ParameterVector are now set using resize() and purrr::walk()
rather than new().

TODO:
* Some additional thought shouldd go into using fromRVector and toRVector
  in future versions of this code. See #766
* Update zzz.R to set methods for RealVector that handle indexing
  translation (e.g., R uses 1-based indexing, C++ uses 0-based indexing).
* Modify the relevant R code that assigns values to RealVector to
  accommodate the updated indexing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
status: triage_needed This is not approved for this milestone, do not work on it yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@kellijohnson-NOAA @Bai-Li-NOAA