flr / FLCore

Core package of FLR, fisheries modelling in R
16 stars 18 forks source link

Converting FLBiol to FLStock fails #50

Closed marchtaylor closed 6 years ago

marchtaylor commented 6 years ago

I have just updated both FLCore and FLBEIA to current versions on GitHub, and am now getting an error that I didn't have before.

I have traced the error to MANAGEMENT PROCEDURE / OBSERVATION MODEL step in FLBEIA; specifically, in the functionFLBEIA:::observation.mp, FLBEIA:::perfectObsis called in my example. Therein is a step where a given FLBiol object is converted to FLStock as such: res <- as(biol, "FLStock")

I get the error: Error in if (!is.na(dms$min) & ran["minfbar"] < dms$min) return("minfbar is lower than first age") : missing value where TRUE/FALSE needed.

I have tried to create a minimal example outside of FLBEIA. I get an error, but it is slightly different:

library(FLCore)  
data("ple4.biol")  
tmp <- as(ple4.biol, "FLStock")
# Error in validObject(res) : invalid class “FLStock” object: 
# All elements must share quant names: Error in FLStock harvest.spwn

Maybe the FLBEIA folks (@dorleta, @ssanchezAZTI) are aware of this issue?

Thanks for your help.

iagomosqueira commented 6 years ago

I have modified coerce(from=FLBiol, 'FLStock') so the spwn slot is expanded to all ages. There was also a problem with range not containing minfbar and maxfbar. These are now set to the min and max ages, so it is worth checking this is correct if fbar() is being called.

Let me know if this creates any problem in FLBEIA.

marchtaylor commented 6 years ago

I have reinstalled FLCore, but still get the same error . Error in if (!is.na(dms$min) & ran["minfbar"] < dms$min) return("minfbar is lower than first age") : missing value where TRUE/FALSE needed Is there anything specific that we should look for in the FLBEIA code that may be preventing this conversion? I will write the FLBEIA folks to have a look.

iagomosqueira commented 6 years ago

We will need to check the method being called is the one recently corrected. Can you email me the simplest possible example code and input objects where this happens?

iagomosqueira commented 6 years ago

The coercion method is returning a valid FLStock

Browse[1]> validObject(res)
[1] TRUE
Browse[1]> n
Error in if (!is.na(dms$min) & ran["minfbar"] < dms$min) return("minfbar is lower than first age") : 
  missing value where TRUE/FALSE needed

but the error persists. Could one of you, @dorleta, or @ssanchezAZTI help me find where in FLBEIA() is this call to as(x, 'FLStock') and see what is being done to the object to being that error message?

marchtaylor commented 6 years ago

I believe this was the line in FLBEIA:::perfectObs that was causing the problem: res <- as(biol, "FLStock")[, 1:(year - 1), 1, 1]

iagomosqueira commented 6 years ago

Can you tell me the file and line where I can find it? Or inside which function? I am having trouble isolating it. Thanks

iagomosqueira commented 6 years ago

OK, got it at

MP_1a_Observation_Model.R:58: res <- as(biol, 'FLStock')[,1:(year-1),1,1]

iagomosqueira commented 6 years ago

OK, problem is in line 61

res@range[c(1:3,6:7)] <- biol@range[c(1:3,6:7)]|
names(res@range[6:7]) <- c('minfbar', 'maxfbar')

biol@range[c(6:7)]| is NA, so minfbar and maxfbar get set to NA. That breaks FLStock validity later on when assigning to stock.n

iagomosqueira commented 6 years ago

The corrected coerce method now sets minfbar and maxfbar to min and max, so maybe those lines are not needed. FLBiol should not have information on fbar in the range slot

Browse[2]>     res <- as(biol, 'FLStock')[,1:(year-1),1,1]
Browse[2]>     dimnames(res) <- list(unit="unique")
Browse[2]> range(res)
      min       max plusgroup   minyear   maxyear   minfbar   maxfbar 
        1         6         6      2014      2016         1         6 
marchtaylor commented 6 years ago

Thanks for identifying this. @dorleta, @ssanchezAZTI - would you be so kind as to take a look at the suggested changes?

iagomosqueira commented 6 years ago

In the fourth call to perfectObs, it tries to do

harvest(res) <- res@catch/(res@stock.n*res@stock.wt)

but res has age=1

while the result of

res@catch/(res@stock.n*res@stock.wt)

has age='all'

A possible solution is to do in here

harvest(res) <- (res@stock.n*res@stock.wt) * (1 / res@catch)

as the units are taken from the first object when mismatched.

This is now becoming an @flr/beia issue, unless the behaviour of FLCore is to blame.

iagomosqueira commented 6 years ago

https://github.com/flr/FLBEIA/pull/23