search

FEniCS / dolfinx

Next generation FEniCS problem solving environment
https://fenicsproject.org
GNU Lesser General Public License v3.0
742 stars 178 forks source link

Make compile time options compile time constants #3246

Closed schnellerhase closed 3 months ago

schnellerhase commented 4 months ago

Resolves #3

garth-wells commented 4 months ago

Looks good - could you add has_ptscotch? (The compiler definition is HAS_PTSCOTCH.)

Seems we had a bug before with the missing definition. SCOTCH was required, but now it's optional, something must have got lost in that change.

schnellerhase commented 4 months ago

I am unsure about what to do with the std::string's. The tested compilers do not seem to have implemented compile time strings yet, so we need to treat them differently.

I tested inline, instead of the consteval which works. Besides that I tried the use of std::string_view which I am unsure is a legal use of such to be honest. Do you have any thoughts on that?

jhale commented 4 months ago

We tend not to manually inline things - let the compiler use its cost model (these are almost certainly already inlined already).

schnellerhase commented 4 months ago

We need either an inline or a consteval to allow for the now multiple definitions in the different compile units. The real in lining of the compiler I do not want to manipulate.

jhale commented 4 months ago

Compile time strings still looks complex in C++20 - just keep those routines as they are.

schnellerhase commented 4 months ago

Compile time strings still looks complex in C++20 - just keep those routines as they are.

In the sense of the current state, or returning them to the previous compiled setting of before?

jhale commented 4 months ago

Can you keep constexpr in the header and keep the string ones in the .cpp?

jhale commented 4 months ago

You should generally avoid using force pushes when working on shared projects.

Could you also add a new wrapper for DOLFINX_STDC_NO_COMPLEX_KERNELS? e.g. has_complex_kernels?

schnellerhase commented 4 months ago

Alright, will refrain from it, just worked with projects that had a linear history before.

I can't seem to find the option DOLFINX_STDC_NO_COMPLEX_KERNELS, can you point me to its definition?

jhale commented 3 months ago

You need to merge origin/main

jhale commented 3 months ago

@schnellerhase could you make sure these are allwrapped into Python in another PR? Also good practice. Thanks.

schnellerhase commented 3 months ago

@schnellerhase could you make sure these are allwrapped into Python in another PR? Also good practice. Thanks.

Got lost somehow, sorry - now done in a PR.