search

ices-tools-prod / icesSAG

R interface to Stock Assessment Graphs database web services
http://sg.ices.dk/webservices.aspx
9 stars 7 forks source link

simplify() is slightly broken #213

Open arni-magnusson opened 3 years ago

arni-magnusson commented 3 years ago

Hi, I'm running this

sag19 <- getSAG(stock=NULL, year=2019)

and the resulting data frame has columns of type character that should (probably) be numeric:

class(sag19$low_SSB)
class(sag19$low_F)

Having these columns as character makes the resulting object larger than necessary (1.7 MB instead of 1.4 MB) and prevents numerical calculations.

It turns out that the utility function simplify() that is used by the parser does not handle negative values correctly, while a similar utility function in icesDatras works fine:

icesSAG:::simplify("-3.14")
icesDatras:::simplify("-3.14")

From a biological viewpoint, low_SSB and low_F should probably not be negative in the first place, but it seems tempting to make the output from getSAG() robust to negative values. One approach could be to revert back to the d408441 version of simplify (same as used in icesDatras), but in case there might be other solutions to consider, I decided to raise this as an issue rather than a pull request.

A smaller reproducible example is:

syt <- getSAG("syt.27.67", year=2019)
class(syt$low_SSB)
cmspinto commented 3 years ago

Hi Arni,

Thanks for your feedback. I can tell you that in the database Low_F and Low_SSB are numeric :)

image

About the possibility to negative values, unfortunately that can happen in simulations. We don't have many, but there are a few cases in SAG where that happens.

arni-magnusson commented 3 years ago

One possible solution is implemented in pull request https://github.com/ices-tools-prod/icesSAG/pull/214