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

Update formula does not work as expected #40

Open
mardukbp opened this issue Aug 24, 2019 · 6 comments
Open

Update formula does not work as expected #40

mardukbp opened this issue Aug 24, 2019 · 6 comments

Comments

@mardukbp
Copy link

In Gnu-R 2.15 and 3.6 update(~1, ~. - y) produces ~1. pqR produces ~1 - 1

radfordneal added a commit that referenced this issue Aug 30, 2019
@radfordneal
Copy link
Owner

Thanks for the report.

I've investigated, and found that the bug is also present in R-2.15.1. It was fixed in R-3.1.1, which reported it as affecting formulas with exactly 32 variables (PR 15735). I've incorporated the R-3.1.1 fix in the development version of pqR. I may put out a minor update with this and other fixes, or it may wait until the new version with automatic differentiation is ready.

@mardukbp
Copy link
Author

Thank you Radford for investigating and fixing this bug. I would say it is better to publish a minor update, which includes this and other bug fixes. That is the purpose of minor updates. And it would allow more people to use pqR as a drop-in replacement for GNU-R.

I came across this project while trying out Oracle's FastR (a JVM-based R implementation) to speed up the performance of the oSCR package. In my only benchmark pqR turned out to be twice as fast as GNU-R and 1.5 times as fast as FastR. Now a friend of mine is using pqR for his research.

I read in the Issues that your enhancements to R are unlikely to be accepted by the R Core Team, which is a pity (to say the least). I suggested to the main FastR developer to incorporate some of your optimizations. It would be great for your hard work to get more recognition and make a wider impact now that R is so fashionable for Big Data and ML. I would also like to note that FastR can be coupled with GraalVM's visual profiler, which makes it pretty easy to detect hot spots in an R package.

@nuest
Copy link

nuest commented Sep 18, 2019

@mardukbp If you still wanna try out this fix in a Dockerfile: Clone https://github.com/nuest/pqr-docker an then run

$ REF=cbc944dabe0818875f99afe298f09430ecb450a2; docker build --tag pqr:$(echo $REF) --build-arg REF=$REF --file dev/Dockerfile .
$ docker run --rm -it pqr:cbc944dabe0818875f99afe298f09430ecb450a2 

pqR version 2.15.1 (2019-00-00), based on R 2.15.0 (2012-03-30)

R 2.15.0 is Copyright (C) 2012 The R Foundation for Statistical Computing
ISBN 3-900051-07-0

Modifications to R in pqR are Copyright (C) 2013-2019 Radford M. Neal

Some modules are from R-2.15.1 or later versions distributed by the R Core Team

Platform: x86_64-unknown-linux-gnu (64-bit)

R is free software and comes with ABSOLUTELY NO WARRANTY.
You are welcome to redistribute it under certain conditions.
Type 'license()' or 'licence()' for distribution details.

R is a collaborative project with many contributors.
Type 'contributors()' for more information and
'citation()' on how to cite R or R packages in publications.

Type 'demo()' for some demos, 'help()' for on-line help, or
'help.start()' for an HTML browser interface to help.
Type 'q()' to quit R.


Used CRAN mirror:  ftp://price.utstat.utoronto.ca/2019-02-19 

No helper threads, task merging enabled, uncompressed pointers.

> sessionInfo()
R version 2.15.1 (2019-00-00)
Platform: x86_64-unknown-linux-gnu (64-bit)

locale:
[1] C

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     
> update(~1, ~. - y)
~1

@nuest
Copy link

nuest commented Sep 18, 2019

Comparison to the commit before the fix:

$ REF=21b7226f26524933ff88ea6335bf807e9c768630; docker build --tag pqr:$(echo $REF) --build-arg REF=$REF --file dev/Dockerfile .
$ docker run --rm -it pqr:21b7226f26524933ff88ea6335bf807e9c768630 Rscript -e 'update(~1, ~. - y)'

Used CRAN mirror:  ftp://price.utstat.utoronto.ca/2019-02-19 
~1 - 1

@mardukbp
Copy link
Author

Thank you Daniel! I already tried the fix on my laptop and on an AWS EC2 instance. Your Docker file would make it easy for people to deploy pqR on AWS. I think this use case should be advertised on the project's website.

@nuest
Copy link

nuest commented Sep 18, 2019

I have no experience with AWS, so if you have a small code chunk or example configuration, I'd be very happy about a PR to the README :-).

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

3 participants