Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
nrlottig opened this issue May 24, 2018 · 10 comments
Open

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

nrlottig opened this issue May 24, 2018 · 10 comments

Comments

@nrlottig
Copy link

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
Copy link
Member

lawinslow commented May 24, 2018

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
Copy link
Contributor

hdugan commented May 24, 2018

Apologies for my terrible 2012 R coding

lawinslow added a commit to lawinslow/LakeMetabolizer that referenced this issue May 24, 2018
@lawinslow
Copy link
Member

@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
Copy link
Author

@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
Copy link
Member

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

@Mkkl-bio
Copy link

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

@hdugan
Copy link
Contributor

hdugan commented Feb 19, 2020

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

jzwart added a commit to jzwart/LakeMetabolizer that referenced this issue Apr 23, 2020
@camilleminaudo
Copy link

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
Copy link
Member

jzwart commented Apr 27, 2020

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
Copy link

These fixes solved all my issues, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants