NOAA-FIMS / FIMS

The repository for development of FIMS
https://noaa-fims.github.io/FIMS/
GNU General Public License v3.0
16 stars 9 forks source link

[Refactor]: ModularTMBExample to more closely mimc FIMS v.0.1.0 #428

Closed Andrea-Havron-NOAA closed 1 year ago

Andrea-Havron-NOAA commented 1 year ago

Refactor request

Currently, the ModularTMBExample repo is a useful test bed for FIMS, however, it was developed primarily to demo the Rcpp interface and its architecture does not mimic that of FIMS (eg. file and naming structure, shared pointers, integer representation managed by an information file). Concurrently, additional modular TMB demos have been developed to test linking TMB functionality into modular C++ code and has been used to test adding REPORT and SIMULATE as well as functionality such as separability, model validation, and TMBad. These latter examples lack an Rcpp interface.

Ideally, these test repos should all be in one location and should be operating on the same toy version of FIMS. A FIMS toy for testing different architecture structures and new features should use a minimalist example yet replicate enough of the FIMS architecture to be comparable.

Should we refactor the ModularTMBExample repo to mimic FIMS v.0.1.0 architecture but with the current minimal working example? Is it better to maintain a collection of toy examples, each with their own purpose?

Concerns/Questions:

Rick-Methot-NOAA commented 1 year ago

My first reaction is that such an example using actual FIMS architecture is valuable if we want to attract other developers. Second is that a modular example does not seem very modular if it is hard to update.

Andrea-Havron-NOAA commented 1 year ago

@Cole-Monnahan-NOAA, tagging you for your perspective given your work with the ModularTMBExample demo

msupernaw commented 1 year ago

@Richard Methot - NOAA Federal @.***> Both are modular, just different architecture. The TMBModular example is much simpler than FIMS and was developed long before any FIMS code was written. The purpose was to demonstrate the idea of using singleton objects to interface with TMB, which has unique architecture in terms of interface requirements. I see Andreas' proposal as worthy and beneficial for the integration team. It won't be a difficult task to refactor the code.

On Tue, Jul 25, 2023 at 11:06 AM Richard Methot @.***> wrote:

My first reaction is that such an example using actual FIMS architecture is valuable if we want to attract other developers. Second is that a modular example does not seem very modular if it is hard to update.

— Reply to this email directly, view it on GitHub https://github.com/NOAA-FIMS/FIMS/issues/428#issuecomment-1650020369, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABFUSEC7DNNM4ITGZLYGOTDXR7OEXANCNFSM6AAAAAA2WHEUQY . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- Matthew Supernaw Scientific Software Developer National Oceanic and Atmospheric Administration Office Of Science and Technology NOAA Fisheries | U.S. Department of Commerce Phone 248 - 396 - 7797

Cole-Monnahan-NOAA commented 1 year ago

I don't see how that example is going to be helpful moving forward and suggest we retire it instead of putting more time into it. In my view it has served its purpose.

msupernaw commented 1 year ago

@Cole Monnahan - NOAA Federal @.***> It has utility and would benefit from continued support. Its simplicity makes it useful for proof of concept tasks. If theTMBModular example mimics the FIMS architecture, we can easily test new ideas and get a feel for issues that may arise in FIMS. I feel it's worth keeping around because it's already proven useful in a handful of ways, whether it's figuring out a TMB mapping solution for FIMS or extracting the declared names from R.

On Wed, Jul 26, 2023 at 11:28 AM Cole Monnahan @.***> wrote:

I don't see how that example is going to be helpful moving forward and suggest we retire it instead of putting more time into it. In my view it has served its purpose.

— Reply to this email directly, view it on GitHub https://github.com/NOAA-FIMS/FIMS/issues/428#issuecomment-1652044536, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABFUSEGIB24MUFZMTSIX33LXSEZSLANCNFSM6AAAAAA2WHEUQY . You are receiving this because you commented.Message ID: @.***>

-- Matthew Supernaw Scientific Software Developer National Oceanic and Atmospheric Administration Office Of Science and Technology NOAA Fisheries | U.S. Department of Commerce Phone 248 - 396 - 7797

Andrea-Havron-NOAA commented 1 year ago

@Cole-Monnahan-NOAA, thanks for your input. My motivation for filing this issue is that I have other architecture questions I would like to explore, especially with respect to the Rcpp interface, and I would like to work on an example that more closely aligns with the FIMS structure. Another way forward is to create different prototypes for each question, but this would lead to duplicated effort and a lot of repos to manage. Do you have a recommendation for a better way to construct a FIMS toy example for R&D?

Cole-Monnahan-NOAA commented 1 year ago

I don't have a recommendation unfortunately. I just question whether modifying and maintaining these examples to do testing like this would actually save time compared to doing the architecture tests in FIMS itself.

Andrea-Havron-NOAA commented 1 year ago

We decided to move forward with this restructure as it isn't too much work and having a toy example will allow testing of structural components, such as pointers, which would otherwise require extensive reworking and troubleshooting if changes were made to FIMS directly.

@msupernaw and I made a lot of progress on changing the ModularTMBExample to more closely match FIMS today. I haven't pushed up my changes yet because I am wondering about the best way to manage the repo

Is it better to push the restructure up to a new repo named for example, FIMSToy, in order to preserve ModularTMBExample in its current state or push changes up to ModularTMBExample directly?

As part of a larger vision for FIMS, would it be better to have a FIMS-RD repo in which the modular example is a component?

k-doering-NOAA commented 1 year ago

Is there a plan to use ModularTMBExample as it is currently in the future? If not, it seems to make sense to add a tag at the current version and then pushing to the same repo.

Andrea-Havron-NOAA commented 1 year ago

@k-doering-NOAA, I don't think there is a plan to use ModularTMBExample in its current state in the future. I like the idea of tagging the current version before pushing up the restructure.

Cole-Monnahan-NOAA commented 1 year ago

I'd push to the ModularTMBExmaple. If someone wanted the old version they could just roll back to that commit.

Andrea-Havron-NOAA commented 1 year ago

See ModularTMBExample, branch FIMS-v0100