Change here-string bashism to POSIX echo

[jeroen-mostert]

The <<< syntax is not POSIX and won't work on systems where sh cares about such niceties (mostly dash). Replace it with a simple echo to get things to work. Note that grepping the CC substring for gcc and clang is probably not a good way of doing these feature checks to begin with (and some flags are duplicated) since AX_CHECK_COMPILER_FLAGS is a thing, but that's left as an exercise to the reader.

Note that configure has to be regenerated with autoconf before this sticks. I haven't made this part of my commit since it causes a lot of churn.

[BiplabRaut]

Hi @jeroen-mostert ,
Thank you for reaching out to us..

We will check on this and get back to you.
In the meanwhile, as a standard practice, it will be good to run the basic sanity, and functional tests.
Here are the recommended steps to run a variety of tests :

1. Configure the library using a combination of configure options like --enable-sse2 --enable-avx --enable-avx2 --enable-avx512 --enable-mpi --enable-openmp --enable-shared --enable-amd-opt --enable-amd-mpifft --enable-dynamic-dispatcher --enable-amd-mpifft --enable-amd-fast-planner --enable-amd-top-n-planner --enable-amd-app-opt --enable-amd-trans

2)Generate a different build of the library based on the configure command

3)Run "make small check" , "make check" "make big check"

Please share/attach the results in this PR itself.

Thanks

[jeroen-mostert]

Alright, I have a few issues with this, so fasten your seat belts. :)

If this is a "standard practice", it's probably a good idea to actually write that down somewhere in a guide for contributors. Of course it's even better to have a build pipeline for it, but that's another kettle of fish.

It is not possible to run ./configure with all options: some are mutually exclusive and some do not apply in double or quad precision mode. It is not really reasonable to require contributors to build and check all possible configurations, as there are too many. You have to choose between the fast planner and the top-N planner for starters, as well as between either MPI mode or the AMD app optimizations, on top of the single, double, long double and quad precision modes.

There is no small or big target. The correct procedure is make -C tests [smallcheck | check | bigcheck]. While smallcheck takes seconds and is fast enough to run after every build as a sanity check, check already takes minutes, and bigcheck takes hours even with hardware acceleration. I would say this is not reasonable to run except when an actual, substantial change has been made to the compute code itself that would warrant it, and before a public release (and in the latter case you should not trust any self-reporting of passed tests by PR authors).

I also assume that by "share the results" you mean sharing the confirmation that the tests passed, and not the entire output of all tests, as this runs in the megabytes for bigcheck and is not particularly insightful.

I suggest the following sequence of commands as a sanity check instead, building all precisions with a reasonable set of options that apply:

./configure --enable-single --enable-sse2 --enable-avx --enable-avx2 --enable-avx512 --enable-mpi --enable-openmp --enable-shared --enable-amd-opt --enable-dynamic-dispatcher --enable-amd-mpifft --enable-amd-fast-planner --enable-amd-trans
make -j ; make -C tests smallcheck

./configure --enable-sse2 --enable-avx --enable-avx2 --enable-avx512 --enable-mpi --enable-openmp --enable-shared --enable-amd-opt --enable-dynamic-dispatcher --enable-amd-mpifft --enable-amd-fast-planner --enable-amd-trans
make -j ; make -C tests smallcheck

./configure --enable-long-double --enable-mpi --enable-openmp --enable-shared --enable-amd-opt --enable-dynamic-dispatcher --enable-amd-mpifft --enable-amd-trans
make -j ; make -C tests smallcheck

./configure --enable-quad-precision --enable-openmp --enable-shared --enable-amd-opt --enable-dynamic-dispatcher --enable-amd-trans
make -j ; make -C tests smallcheck

After each of these, it should be verified that these messages appeared:

--------------------------------------------------------------
         FFTW transforms passed a few tests!
--------------------------------------------------------------
--------------------------------------------------------------
         FFTW threaded transforms passed a few tests!
--------------------------------------------------------------

Even this takes a substantial time on the first compile, but ccache can help greatly in speeding it up on subsequent builds. For a very quick sanity check, only the first command can be used, testing only the single precision build but with all bells and whistles.

And yes, to be complete: I have verified my changes pass these tests, trivial as they are. :P

[BiplabRaut]

Thank for your detailed enthusiastic response.

We are planning to publish a guide for contributors pretty soon and also add a CI pipeline in the near future.
For your pending patch, I suggested to run a set of tests by configuring the library in different ways depending upon the
combination of configure options.

A few combinations of configure options are not permitted for use together and the user shall avoid them based on the error message.

In my previous message, I missed to remove space from "make small check" and "make big check", but a general FFTW user would know.

You can share the test completion status instead of the full logs.

[e-kwsm]

Aren’t [[/]] bashism as well?

[jeroen-mostert]

Yes, you're quite right. To be fair I spent exactly as much effort as needed to get things to run on a distro that used dash for sh and nothing more, and the clang branch was never hit. If I have copious free time I can check the whole file for bashisms (shellcheck does not pick up on this, for some reason).

[e-kwsm]

Hopefully these bashism stuffs can be found in Ubuntu GitHub Actions once implemented.