Add population derived quantity lookup by name in rcpp#1357
Open
codeafridi wants to merge 2 commits into
Open
Conversation
Contributor
|
@nathanvaughan-NOAA and @Andrea-Havron-NOAA are you using parts of this PR for your refactor on random effects? If yes, I would like to ensure that @codeafridi gets credit by being a commit author. We can modify this PR if that would be helpful. Please let me know how to proceed. Also, @msupernaw are there parts of the PR that you think should be brought into the codebase? |
Contributor
|
Not sure at the moment @Kelli Johnson - NOAA Federal
***@***.***> but we will make sure to attribute it correctly if
we do.
*Nathan R. Vaughan Ph.D.*
Research Scientist, Contractor/Affiliate with Vaughan Analytics in support
of
NOAA Fisheries - Southeast Fisheries Science Center | U.S. Department of
Commerce
Mobile: (305) 733-6643
www.fisheries.noaa.gov
…On Tue, May 19, 2026 at 9:49 PM Kelli Johnson ***@***.***> wrote:
*kellijohnson-NOAA* left a comment (NOAA-FIMS/FIMS#1357)
<#1357 (comment)>
@nathanvaughan-NOAA <https://github.com/nathanvaughan-NOAA> and
@Andrea-Havron-NOAA <https://github.com/Andrea-Havron-NOAA> are you using
parts of this PR for your refactor on random effects? If yes, I would like
to ensure that @codeafridi <https://github.com/codeafridi> gets credit by
being a commit author. We can modify this PR if that would be helpful.
Please let me know how to proceed. Also, @msupernaw
<https://github.com/msupernaw> are there parts of the PR that you think
should be brought into the codebase?
—
Reply to this email directly, view it on GitHub
<#1357 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMU2O2VV3RTQRCI2ZP7E6VD43UFLZAVCNFSM6AAAAACW62E3U2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DIOJTG44DQMJSHE>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
proof of concept for issue #1291
What is the feature?
This PR adds a small proof of concept for referencing a population derived quantity from R by name. in issue #1291, the main problem is that derived quantities are created during model setup but do not have a consistent R facing access path like parameters do so I focused on a minimal population side example.
How have you implemented the solution?
I added a small registry path in the CatchAtAge Rcpp interface for population derived quantities during model initialization.
The change does the following:
For the proof of concept, I used "spawning_biomass".
I also added a focused test in "test-initialize_modules.R" to check that:
Does the PR impact any other area of the project, maybe another repo?
No this change is only within FIMS