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

Fix Sundials make config #152

Merged
merged 4 commits into from
Jul 6, 2023
Merged

Fix Sundials make config #152

merged 4 commits into from
Jul 6, 2023

Conversation

aseyboldt
Copy link
Contributor

@aseyboldt aseyboldt commented Jul 6, 2023

Previously there was no test that needed correct linking of sundials (the ode test was only using the Runge Kutta solver).
This fails locally for me, but I would like to see if it also does so in the CI, or if this is just an issue on my machine.

It is possible that the makefile config that leads to incorrect sunidals linking is also responsible for the compile times in #151.

Update: It also fails in the CI. Here it looks like the linux tests might pass, but locally they fails for me when the module is loaded (for instance in the python tests).

@WardBrian
Copy link
Collaborator

This was caused by stan-dev/math#2885 (review), the fix is to change LIBSUNDIALS to SUNDIALS_TARGETS in our makefile

@aseyboldt
Copy link
Contributor Author

That does seem to fix the issue :-)
I had to manually set CXXFLAGS_SUNDIALS += -fPIC, is it expected that CXXFLAGS is not used in the sundials build commands?

@WardBrian
Copy link
Collaborator

Let me look into CXXFLAGS. We inherit all that from the stan-dev/math makefiles, and they can be a bit opaque.

I think we can delete the existing RK45 ode test - I didn't check that the solver was actually from sundials, which is my bad.

@WardBrian WardBrian changed the title test: Add a test that uses sundials Fix Sundials make config Jul 6, 2023
@WardBrian WardBrian merged commit 7bf32a4 into roualdes:main Jul 6, 2023
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.

2 participants