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

Changes needed for future rstan #460

Closed
wants to merge 6 commits into from
Closed

Changes needed for future rstan #460

wants to merge 6 commits into from

Conversation

andrjohns
Copy link

Hi,

This PR adds the changes to compiler and linker flags needed for the upcoming version of rstan/StanHeaders. All that was needed in this case was adding the RcppParallel include files and linker flags.

Let me know if you need any more info.

Thanks!
Andrew

@mattfidler
Copy link
Collaborator

Thanks @andrjohns

Running other checks. It is likely just fine

@mattfidler
Copy link
Collaborator

Somehow this fails.

@mattfidler
Copy link
Collaborator

I am re-running the base to see if it still works

inst/tools/workaround.R Show resolved Hide resolved
@andrjohns
Copy link
Author

Sorry about that, I updated the call to StanHeaders:::LdFlags() to satisfy Codefactor but it caused a different error. I've submitted the fix now

@andrjohns
Copy link
Author

I've re-run these changes locally on Windows and I can't replicate the R CMD check error from the windows testing above, would you be able to test locally as well?

@andrjohns
Copy link
Author

Apologies that this has been such a process. I was able to replicate the failure, it was caused by Windows linking against the wrong architecture for RcppParallel (i.e., was trying to link to 64-bit during 32-bit compilation)

@andrjohns andrjohns closed this Oct 10, 2021
@mattfidler
Copy link
Collaborator

Thia isnt needed anymore?

@andrjohns
Copy link
Author

Apologies, thought I'd clicked close with comment. It's still needed, but I'll debug the errors on my fork rather than clogging up your PRs/actions

@mattfidler
Copy link
Collaborator

Ok. 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

Successfully merging this pull request may close these issues.

None yet

2 participants