[Refactor]: when should FIMS switch to TMBad?

[Andrea-Havron]

Refactor request

TMB's default AD library depends of CppAD but a new AD library, TMBad, is available for TMB version 1.8.0 and hgiher.

Currently, the FIMS Rcpp Interface sets up four instances of the model to interface with CppAD Types (defined here). This architecture does not work when using the TMBad library.

Should FIMS develop an interface that works with both libraries or deprecate the support for CppAD, requiring users install TMB 1.8.0 and higher?
TMBad is still considered experimental. What criteria should we use to determine TMBad is tested enough to use for FIMS?

Expected behavior

Currently when trying to run the ModularTMBExample with the DTMBAD_FRAMEWORK compile flag specified, the model compiles and runs, but parameter values are not being updated and the objective function is not minimized.

A new Rcpp interface needs to be specified when #ifdef TMBAD_FRAMEWORK. Optimization needs to be implemented in order to test the architecture is set up correctly for the new TMBad library to work.

[msupernaw]

We should develop an interface that works with both libraries and eventually deprecate the former. After TMBad is out of experimental mode, we can use an R script to rewrite the Makevars files and expose the appropriate #define statements based on the installed version of TMB.
Something like this:

v<-compareVersion(toString(packageVersion("TMB")), "1.9.0")

if(v == 1){

 file.rename(from = "~/.R/Makevars", to = "~/.src/Makevars_old")
 file.remove("~/.src/Makevars")

 file.rename(from = "~/.R/Makevars.win", to = "~/.src/Makevars.win_old")
 file.remove("~/.src/Makevars")

 cat("CXX14FLAGS +=  -DTMB_AD_MODEL -mtune=native -march=native -Wno-ignored- 
 attributes -Wno-deprecated-declarations", file = "~/.src/Makevars")

 cat("CXX14FLAGS +=  -DTMB_AD_MODEL -mtune=native -march=native -Wno-ignored- 
 attributes -Wno-deprecated-declarations", file = "~/.src/Makevars.win")

}

[k-doering-NOAA]

I agree with the philosophy of testing out TMBad and then once we are confident it is working well with FIMS, deprecating CPPad so we don't need to maintain CPPad code in the long term. I think the deprecation process can be very quick given FIMS is still in the early stages of development and no one is depending on it yet for stock assessments (or other uses).

[ChristineStawitz-NOAA]

Speedups come from TMBad - Kasper speeds up the AD computations with a list. Bias correction is faster due to some rank-reduced thing. It would make add_to_fims_tmb simpler because we would only need one template rather than four. At some point, Kasper will deprecate the CppAD and then we will need to switch, keeping reverse compatibility with CppAD would increase flexibility without much overhead but would increase maintenance costs.

[Andrea-Havron-NOAA]

Related to Issue #390 #241

[kellijohnson-NOAA]

This issue is currently in the "Parking Lot" milestone but I would like to make a decision if we should switch to TMBad. Please, view this comment on GitHub and respond with a thumbs down if you do NOT want to move to TMBad. If there are any 👎 or 👀 reactions by EOD June 21, 2024, I will schedule some time, for those that want to attend, to discuss the decision.

[Andrea-Havron-NOAA]

TMBad functinality would require:

• Adding the three lines of code from tmb_core.hpp to FIMS def.hpp.
• Each rcpp_object would need a new line specifying the TMBad type (e.g. here).
• CreateTMBModel() in rcpp_interface updated to point to the single GetInstance from TMBad
• clear_internal in rcpp_interface updated to clear TMBad pointers
• Old Cppad specific code would need to be wrapped in #ifdef CPPAD_FRAMEWORK and new TMBad code wrapped in #ifdef TMBAD_FRAMEWORK
• Makevars changed to include -DTMBAD_FRAMEWORK flag (e.g. here)

[msupernaw]

👎, my vote is to wait until M2 is complete. I feel we should focus on the modeling components of FIMS. Switching would be trivial if there are no problems, but if there are issues discovered, it might hinder development if we devote time to a TMB problem.

[kellijohnson-NOAA]

As discussed in NOAA-FIMS/seaside-chats#5, we will be switching to TMBad after M2 during the cleanup phase.

[msupernaw]

Good, I missed that! Thanks.

[Andrea-Havron-NOAA]

@kellijohnson-NOAA, should we add this task to #662?