NOAA-FIMS / collaborative_workflow

contributors guide to FIMS, managing collaborations
https://noaa-fims.github.io/collaborative_workflow/
4 stars 1 forks source link

[Feature] File roadmap for where to put new functions and code #77

Closed ChristineStawitz-NOAA closed 9 months ago

ChristineStawitz-NOAA commented 2 years ago

Is your feature request related to a problem? Please describe. It's unclear where to put Rcpp modules vs math functions vs C++ classes when adding code

Describe the solution you'd like A roadmap for developers to make it clear which folders and header files to put their code in

ChristineStawitz-NOAA commented 2 years ago

commit 66bd700 starts to address this, but I'd like to use the issue thread to discuss the ideal folder layout for FIMS. I agree with the feedback that the current layout feels duplicative.

To my understanding, a minimal FIMS model should have a math library, population dynamics functors, and a TMB objective function structured as follows:

Proposed changes

Using selectivity as an example, I understand why there is a logistic function in fims_math.hpp and a SelectivityBase class that is a parent class to LogisticSelectivity, but I don't understand why instead of a single selectivity.hpp file that implements those two classes, there is a functors folder with two .hpp files inside it and then another selectivity_base.hpp that is empty other than #include statements.

Andrea-Havron-NOAA commented 2 years ago

With respect to your question about the duplication between selectivity.hpp and functors/selectivity_base.hpp, I think this duplication is used to organize/group include statements. From reviewing the FIMS/software_arch repository, the selectivity.hpp should include all .hpp files in the functors folder (it currently doesn't which needs to be fixed). This way, we only need to #include population_dynamics/selectivity.hpp in the header of the model.hpp and we read in all the .hpp files in the selectivity functors folder. The functors folder will eventually include the selectivity_base.hpp along with all other selectivity functions we want to implement in FIMS.

ChristineStawitz-NOAA commented 9 months ago

We think a lot of this is addressed in the vignette @Andrea-Havron-NOAA created. Closing but we can start a new issue if we need a clearer roadmap: https://noaa-fims.github.io/FIMS/articles/fims-demo.html#creating-modules-in-fims