NOAA-FIMS / FIMS

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

[Refactor]: add tests on whether a fleet is coded as a survey #284

Open Andrea-Havron-NOAA opened 1 year ago

Andrea-Havron-NOAA commented 1 year ago

Refactor request

This issue is to flag comments made about treating surveys as fleets:

@Andrea-Havron-NOAA asked in a previous PR about whether treating surveys as fleets means we will have an F for surveys. @cmlegault reminded the team about the issue with treating surveys as fleets is that then you need to account for surveys in allocation @ChristineStawitz-NOAA mentioned it is not clear if the above will affect milestone 1 integration test or only subsequent milestones

Update 6/8/23: @Andrea-Havron-NOAA noted that this produces an issue with setObservedIndexData in the Rcpp interface, specifically fishery catches currently have to be set as an index The subgroup charged with code reviewing the fleet module (Cole/Jane/Huihua/Bai/Andrea) decided this should be a M2 refactor and agrees that surveys should be a separate module from fleets. Consideration should be given to how code can potentially be shared between the separate modules to reduce repetition, while honoring the fundamental differences between a fishery fleet and survey. Here's an instance where the age comp likelihood had to be hardcoded because of how expected age comps are calculated for both surveys and fleets.

Expected behavior

A decision needs to be made about if the current structure of survey as fleets is sufficient for M1 or needs to be updated.

ChristineStawitz-NOAA commented 1 year ago

Thanks for flagging this @Andrea-Havron-NOAA. In integration testing I would expect this to come out as total F being calculated correctly but the fishing F from FIMS being slightly lower than what is in the model comparison project, and I suspect we're going to have bigger fish to fry than slight mismatches when we start integration testing. I would like to look at this again once we have the integration test running so I'm going to leave in M1 for now.

ChristineStawitz-NOAA commented 1 year ago

Looking like we have matching outputs despite surveys being treated as fleets, so tentatively moving to M2

Bai-Li-NOAA commented 1 year ago