search

lkerr / groundfish-MSE

Development of Robust Management Strategies for Northeast Groundfish Fisheries in a Changing Climate
MIT License
4 stars 2 forks source link

sumCW computed twice? #244

Closed mle2718 closed 2 years ago

mle2718 commented 2 years ago

@jerellejesse found this. It looks like sumCW is computed twice. First, runSim calls get_J1updates() https://github.com/lkerr/groundfish-MSE/blob/ea59c71d82db906bd7f280c4390809ba0b5c8c86/processes/runSim.R#L74-L76 which in turn does: https://github.com/lkerr/groundfish-MSE/blob/ea59c71d82db906bd7f280c4390809ba0b5c8c86/functions/popdy/get_J1Updates.R#L74-L78

I'm a little surprise this part runs, because it looks like it takes at least 1 argument F_full[y] that may be Na or NaN. F_full[y] is set in get_advice(), which is run a little later.

Later on, runSim calls get_mortality() https://github.com/lkerr/groundfish-MSE/blob/ea59c71d82db906bd7f280c4390809ba0b5c8c86/processes/runSim.R#L128-L131 which does: https://github.com/lkerr/groundfish-MSE/blob/ea59c71d82db906bd7f280c4390809ba0b5c8c86/functions/popdy/get_mortality.R#L13-L18

I don't believe this affects anything and i suspect it's a holdover from when @samtruesdell hacked things apart to accommodate the economic model.

samtruesdell commented 2 years ago

As far as I can tell this is a repeat ... I probably meant to include get_mortality in the get_J1Updates or something. I don't think your pull request will present an issue; I think you're right that it just overwrites with the same information. I would definitely would be good with merging this pull request if a couple tests don't flag any issues.