flatironinstitute / finufft

Non-uniform fast Fourier transform library of types 1,2,3 in dimensions 1,2,3
Other
291 stars 73 forks source link

Use std::exp in examples with c++11 #537

Closed DiamonDinoia closed 3 weeks ago

DiamonDinoia commented 1 month ago

the examples seems to require c++14 on mac because we use exp instead of std::exp. We should downgrade to c++11

ahbarnett commented 4 weeks ago

Can you clarify your proposal? Currently the lib is c++17 to build. Makefile builds examples also with c++17 in CXXFLAGS. What does cmake build examples with? Example sources use using namespace std; which isn't great, but surely this gives access to std::exp, right?

So I need clarification: are you proposing to remove std namespace from all exmaples and write std:: for all math functions?

DiamonDinoia commented 4 weeks ago

While finufft itself requires c++17 once the library is built it can be used in any std standard. The way we wrote the examples seems to cause a problem on mac when using c++11 for the examples due to the way we use the exp function. Checking what is going on and fixing it so that users can install a pre built libinufft and use it with older compilers that do not support c++14 or 17 seems a good idea.

ahbarnett commented 4 weeks ago

so I still need a suggested fix. is removing "using namespace std" in all examples necessary?

On Thu, Sep 5, 2024 at 12:30 PM Marco Barbone @.***> wrote:

While finufft itself requires c++17 once the library is built it can be used in any std standard. The way we wrote the examples seems to cause a problem on mac when using c++11 for the examples due to the way we use the exp function. Checking what is going on and fixing it so that users can install a pre built libinufft and use it with older compilers that do not support c++14 or 17 seems a good idea.

— Reply to this email directly, view it on GitHub https://github.com/flatironinstitute/finufft/issues/537#issuecomment-2332171916, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACNZRSXYCHCJ7RJ6P2FRMLLZVCBH7AVCNFSM6AAAAABNAHJJT2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMZSGE3TCOJRGY . You are receiving this because you commented.Message ID: @.***>

-- *-------------------------------------------------------------------~^`^~._.~' |\ Alex Barnett Center for Computational Mathematics, Flatiron Institute | \ http://users.flatironinstitute.org/~ahb 646-876-5942

DiamonDinoia commented 4 weeks ago

The problem is:

// allows 1i to be the imaginary unit... (C++14 onwards)
using namespace std::complex_literals;

So my original diagnosis was wrong.

DiamonDinoia commented 3 weeks ago

Feels more work than what is worth.