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

Minimal changes to use modified Fields behaviour in RcppArmadillo #4

Closed
eddelbuettel opened this issue Feb 26, 2022 · 2 comments
Closed

Comments

@eddelbuettel
Copy link

Please apply these two one-line changes to src/Makevars.in and src/Makevars.win

diff --git a/src/Makevars.in b/src/Makevars.in
index 7fd474c..cc4140a 100644
--- a/src/Makevars.in
+++ b/src/Makevars.in
@@ -1 +1,2 @@
-PKG_LIBS = $(LAPACK_LIBS) $(BLAS_LIBS) $(FLIBS) @OPENMP_CFLAGS@
\ No newline at end of file
+PKG_CPPFLAGS = -DRCPP_ARMADILLO_FIX_Field
+PKG_LIBS = $(LAPACK_LIBS) $(BLAS_LIBS) $(FLIBS) @OPENMP_CFLAGS@
diff --git a/src/Makevars.win b/src/Makevars.win
index 22ebc63..d441b87 100644
--- a/src/Makevars.win
+++ b/src/Makevars.win
@@ -1 +1,2 @@
+PKG_CPPFLAGS = -DRCPP_ARMADILLO_FIX_Field
 PKG_LIBS = $(LAPACK_LIBS) $(BLAS_LIBS) $(FLIBS)

They each just add the line PKG_CPPFLAGS = -DRCPP_ARMADILLO_FIX_Field to ensure each compilation defines this declaration (and doing so is easier/quicker than adding it to each source file).

However, when you do, your behaviour in the package changes and examples and tests fail. Can you look into adapting the code?

If you can't or won't because the old (long-published but arguably 'buggy') behaviour by RcppArmadillo was useful for your case we can simply do the inverse and define the declaration of 'yes please use old RcppArmadillo behaviour' as describe in issue over in its repo . You are probably in a better position to judge how this should evolve.

Please let me know if I can help otherwise, I simply haven't had time to dig into internals of your package yet.

@esmucler
Copy link
Owner

Dirk, thanks for taking the time to go over this. I tried it out and indeed the tests fail, and I cannot figure out what goes wrong at the moment. I am pretty sure that the old "buggy" behaviour made my code behave as I intended, so for now I am opting to put the alternative declaration in place. I tried this out with RcppArmadillo 0.10.8.2.0 and everything seems to work out nicely; see here.

I guess if I submit this new version to CRAN this whole thing will be resolved for odpc, right?

Best,
Ezequiel

PS: Also, thanks a lot for developing Rcpp and RcppArmadillo, they are fantastic.

@eddelbuettel
Copy link
Author

Yes -- If you want to leave things 'as is' we need to add the 'opt-into' as you have done in the updated #3 and it only needs to be at CRAN before May when I plan to 'flip the switch'.

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

2 participants