-
Notifications
You must be signed in to change notification settings - Fork 5
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 wrong pointer passed to Fortran MPI in FFTE #37
Conversation
CI seems to be saying something different than my build. I've run the FFTE test with the same set of packages and got no issues. It seems that the build inside CI doesn't get the latest patch and runs the old code that segfaulted before. Here's my output:
|
You are correct. The CI uninstalls the fiber package at the begining of the job, but doesn't touch the previously installed dependencies. I will update the CI so that it will uninstall all the FFT libraries as well.
|
Correction: The CI is supposed to be uninstalling the fiber and fft packages, but it doesn't appear to be doing so. I'm investigating the cause. |
OK, the CI is now able to uninstall fiber and the fft library before attempting to install and test. I will run the CI checks on this PR. |
external/ffte/tools/CMakeLists.txt
Outdated
@@ -0,0 +1,50 @@ | |||
cmake_minimum_required(VERSION 3.8) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For uniformity, I would suggest to keep the same 3.10 version as in the main CMakeList?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not for FIBER. This is for those who build FFTE with CMake. FFTE provides only Makefile's for their tests and nothing else. FFTE doesn't need a new CMake version than 3.8 that has features for libraries and for CUDA language.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed d5d4896 to make them uniform
Can you merge the changes that I recently made to the CI into your branch in order for the test to run with the updated CI config? The changes are under .github/ |
It took me forever to merge your CI branch. I hope it was worth it. |
I'd be okay to close this and start from scratch after the tests finish. |
Closing due to wrong |
This fixes the outstanding problem from #13 and shows how to call other functions from FFTE backend implementation.