support FFTW3_LIBS FFTW3_INC_DIRS FFTW3_LIB_DIRS in FFTW3 ExternalLibrary

Create issue
Issue #1735 closed
Roland Haas created an issue

Right now FFTW3 claims to support FFTW3_LIBS (in configuration.ccl) but ignores the user provided value and always uses fftw3, it does not allow FFTW3_INC_DIRS or LLTW3_LIB_DIRS to be set at all.

The attached patch changes this such that if those variables are specified in the option list, they override whatever detect.sh may want to use. This can be used to make use of the fftw implementation inside of MKL which stores its include files inside $MKL_ROOT/include/fftw and which should be used by sepcifying jsut `-mkl}} to the compiler rather than an actual library file.

This is a bug since FFTW3_LIBS is documented but does not behave as documented. It is major since this should be fixed before the next release.

Keyword: FFTW3

Comments (9)

  1. Ian Hinder
    • changed status to open
    • removed comment

    I had to go and look up the meaning of this syntax. The ":" prefix means "don't do anything with the return value, just do the side effects of the command" (http://stackoverflow.com/questions/7444504/explanation-of-this-use-of-the-colon-operator). The difference between := and = in the parameter expansion is that := changes the value if the parameter is unset or null, whereas = only changes it if it is unset (https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html). I am assuming that the act of declaring the options in configuration.ccl causes Cactus to set the variables to null, which explains the need for all the changes in the patch. Assuming I have understood this correctly, please apply.

  2. Roland Haas reporter

    Uhm, actually I was mistaken in my usage of ":=" then. I had not thought about ${parameter=word} setting parameter if it is undefined at all. Having said that, I would actually prefer the behaviour of ${parameter=word} over ${parameter:=word} so that one can eg define an empty FFTW3_LIBS setting. So I'd like to amend the patch to only modify configuration.ccl and it is no longer a bug but just an enhancement.

    Still ok to apply?

  3. Ian Hinder
    • removed comment

    I'm not following this fully. In the description, you said that it currently ignores FFTW3_LIBS, and presumably your patch fixed this. Is that not correct?

  4. Roland Haas reporter
    • removed comment

    That was indeed an incorrect statement. I had read:

    FFTW3_LIBS="fftw3"
    

    instead of

    : ${FFTW3_LIBS="fftw3"}
    

    since I was completely unaware of the form ${parameter=word} and only knew of ${parameter:=word} so completely mis-parsed the code based on patterns of code that I expected to find. Bash's syntax is really quite strange, in particular since ":" is basically the command "true" so really we only need side effects and any do-nothing shell command would do.

    Being able to specify "odd" include directories (that are not called "include") from the option list is still required for using MKL though, so that configuration.ccl changes are still something I'd like to commit.

  5. Ian Hinder
    • removed comment

    Yes please. I will start asking in future if patches in review have been tested... :)

  6. Roland Haas reporter
    • changed status to resolved
    • removed comment

    Thank you. The patch was tested. I works as advertised in allowing me to use MKL's fftw3 on zwicky using Intel 14. I was the bug that had not been verified (since the issue started out with me wanting to add FFTW3_INC_DIRS).

    This way at least the patch actually got better due to review and was not just waved through :-) .

    Applied in rev 47 of FFTW3.

  7. Log in to comment