-
Notifications
You must be signed in to change notification settings - Fork 9
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
645 add variable map #665
645 add variable map #665
Conversation
issue #645 add variable_map infrastructure * add variable map as an object in informatin.hpp * add std::vector<uint32_t> key to density_components_base * add unique ID to VariableVector class and get_id function * resize key and pass to densities in rcpp_distributions each rcpp_object: set unique key for all tracked parameter/derived value * rcpp_fleet: log_Fmort, expected_catch, expected_index, catch_numbers_at_age * rcpp_population: log_init_naa, log_M, numbers_at_age * rcpp_recruitment: logit_steep, log_rzero, log_sigma_recruit, log_devs * rcpp_selectivity: inflection_point, slope, inflectin_point_asc, slope_asc, inflection_point_desc, slope_desc refactor the following vectors to use .estimated_m and .is_random_effect_m in interface: * fleet->log_Fmort * population->log_M * population->log_init_naa * recruitment->log_devs add documentation refactor user interface * fix tests to work with C++ changes
Instructions for code reviewerHello reviewer, thanks for taking the time to review this PR!
Checklist
|
@Bai-Li-NOAA , can you review changes I made to |
inst/include/distributions/functors/density_components_base.hpp
Outdated
Show resolved
Hide resolved
@Andrea-Havron-NOAA, would you like me to review only the changes in the tests or all the changed files? I started reviewing all the files but realized that it might take some time. I'm also finding that reviewing some of the .hpp files is stretching my current knowledge🤔. Could we possibly get another pair of 👀 on the pull request or split the review effort for some of the files? Thank you! |
@Bai-Li-NOAA, we have already done an initial review of the code except for the changes I made to the two files: There will also be a more in depth review on the PR from dev to main. I just need you to review the two files listed above. Thanks! |
@Andrea-Havron-NOAA, the |
* issue #646 * refactor model.hpp * refactor loops over fleet and population * loop over all density modules to sum up joint negative log-likelihoods * refactor fleet.hpp * remove evaluate_age_comp_nll * remove evaluate_index_nll in fleet.hpp * remove log_obs_error from fleet * refactor distributions * add conditional to handle data object * add distribution logs * reset nll_components to zero * refactor interface * remove estimate booleans from fleet and population * remove set likelihood components from fleet * refactor data objects to be set from distributions interface * expose vectors to R * track parameter names * fix C++ tests to work with changes in C++ code * refactor user interface * fix R tests to work with changes in C++ code * remove test on nll in recruitment * report out nll components * set log devs estimation to true * update demo to work with changes in C++
544cc9f
to
a02fd5a
Compare
What is the feature?
How have you implemented the solution?
each rcpp_object: set unique key for all tracked parameter/derived value
refactor the following vectors to use .estimated_m and .is_random_effect_m in interface:
add documentation
refactor user interface
refactor model.hpp
refactor fleet.hpp
refactor interface
fix C++ tests to work with changes in C++ code
refactor user interface
refactor distributions
Does the PR impact any other area of the project, maybe another repo?
The user will now set up distributions through the R interface