search

GLEON / LakeMetabolizer

Collection of Lake Metabolism Functions
16 stars 10 forks source link

k.read.base doesn't work with vector data in R v3.5.0 #129

Open nrlottig opened 6 years ago

nrlottig commented 6 years ago

I've been working on training up an undergrad to work on processing LTER metabolism data. I had her go through the workshop materials @hdugan provided at GLEON a couple of years ago. When trying to use k.read.base, she got an error about coercing to dbl. I tried the code on my mac and it ran fine with R v3.4.4. I upgraded to 3.5.0 and it kicked back the same error, downgraded and it ran fine. Downgrade the PC to v3.4.4 and it ran fine. Here is a link to our saved workspace and the script. Line 76 is the call to k.read.base that fails in v.3.5.0

https://www.dropbox.com/sh/xh2xhp5uh8cpztd/AAB6hRzYJ-F4I3PNn7fqVevla?dl=0

lawinslow commented 6 years ago

Thanks @nrlottig. I confirm this is an issue in R v3.5 and not in R v3.4 Looks like its related to this line. We are creating a data.frame and then numerically treating it like a vector. Interestingly, it seems to work up until, I think, the point where we try to raise it to an exponent here. Probably just needs to be broken as a vector and treated as such.

hdugan commented 6 years ago

Apologies for my terrible 2012 R coding

lawinslow commented 6 years ago

@nrlottig could you please test and confirm the fix. To test, install from my fork

install.packages('devtools')
devtools::install_github('lawinslow/LakeMetabolizer')

Please let us know if that solved the v3.5 issue!

nrlottig commented 6 years ago

@lawinslow it runs as expected now on my mac with v3.5. Do you want us to run this version of LakeMetab as we process stuff to see if we find anything else. We can always keep one computer downgraded to 3.4 to check.

lawinslow commented 6 years ago

Yup, go for it. If you find more bugs post them here.

Mkkl-bio commented 4 years ago

I find the same issue when running the k600 demo included in the package. image

hdugan commented 4 years ago

Luke's update works, just needs to be pulled into the parent repo. Also, need the same kinematic viscosity fix for: k.read.soloviev.R k.macIntyre.R

camilleminaudo commented 4 years ago

Hi, I encountered the same issue with k.read(), k.read.soloviev() and k.macIntyre(), even when working with the data provided as an example for the package pulled as follows. data.path = system.file('extdata', package="LakeMetabolizer") tb.data = load.all.data('sparkling', data.path) ts.data = tb.data$data

It only worked if I compute inside a loop on the temporal dimension, timestep after timestep. I then followed @lawinslow 's response from May 24th 2018 (see above): devtools::install_github('lawinslow/LakeMetabolizer') and it solved the issue for k.read()

However, this issue persists for k.read.soloviev.base() and k.macIntyre.base() Do I need to install the package again but from a different GitHub fork? Is this related to the kinematic viscosity calculation that was edited for k.read() but not for the other methods? Thanks in advance for your help! Camille

jzwart commented 4 years ago

Hi @camilleminaudo , We added some fixes to the code here that should clear up these issues. It's merged into the master branch of GLEON/LakeMetablizer so you should be able to install the fixes with: devtools::install_github('GLEON/LakeMetabolizer')

We'll get up the newest version on CRAN soon.

Let us know if there are any existing issues. Jake

camilleminaudo commented 4 years ago

These fixes solved all my issues, thanks!