core icon indicating copy to clipboard operation
core copied to clipboard

Update doc for adaptVerbose and mark it deprecated

Open joshia5 opened this issue 1 year ago • 8 comments

There are differences in adapt and adaptVerbose function as it is now (apart from printing out the mesh and other print statements) which include additional iteration(s) of refine+snap https://github.com/SCOREC/core/blob/8959c599cc05e21d0fb470f941d9f892da62aa02/ma/ma.cc#L106-L107
and calling fixelementShapes Niter times https://github.com/SCOREC/core/blob/8959c599cc05e21d0fb470f941d9f892da62aa02/ma/ma.cc#L85 This PR attempts to improve the function documentation in the header stating the above-mentioned differences and mark function deprecated for potential upcoming name-change (breaking)

joshia5 avatar Aug 16 '24 16:08 joshia5

@joshia5 @cwsmith you may want to check if intel support the C++14 standard deprecated attribute

see: https://en.cppreference.com/w/cpp/language/attributes/deprecated

If it doesn't you could define a PUMI_DEPRACATED macro that's compiler dependent.

jacobmerson avatar Aug 20 '24 02:08 jacobmerson

Yeah, I like the idea of having the macro. Unfortunately, I don't see the 'deprecated' attribute (old or new C++14) listed in the oneAPI Intel compiler reference (linked above).

cwsmith avatar Aug 20 '24 10:08 cwsmith

C++ 17 guarantees

All attributes unknown to an implementation are ignored without causing an error. https://en.cppreference.com/w/cpp/language/attributes

it may be worth checking if intel safely ignores [[deprecated]]. Or, are we not compiling core in 17 mode yet?

jacobmerson avatar Aug 21 '24 05:08 jacobmerson

We require C++11 but optionally enable C++14. Our local systems don't currently have access to the new Intel oneAPI compilers, but it looks like we may be able to download them: https://www.intel.com/content/www/us/en/developer/tools/oneapi/dpc-compiler-download.html?operatingsystem=linux&distribution-linux=offline

cwsmith avatar Aug 21 '24 13:08 cwsmith

I know this got forgotten a little bit, but we can use CMake try_compile and create a macro that either evaluates to [[deprecated]] if supported or nothing otherwise.

bobpaw avatar Mar 03 '25 21:03 bobpaw

@bobpaw I think that would work. My only concern with 'test compilation' to detect feature support is the added complexity when we have to cross compile. We haven't had a machine that requires that in a few years but I'm sure we will get more.

Towards testing if the Intel OneAPI compilers will ignore/support the flag, it looks like we could install the oneapi compilers into a github hosted runner Ubuntu VM (https://neelravi.com/post/intel-oneapi-install/) or possibly via binary installer on SCOREC.

Alternatively, since we are preparing the 4.0 release in #425 , we could include the meshadapt API change as @bobpaw suggested in a thread on that PR. That may be the easiest path forward.

cwsmith avatar Mar 04 '25 16:03 cwsmith

@bobpaw I think that would work. My only concern with 'test compilation' to detect feature support is the added complexity when we have to cross compile. We haven't had a machine that requires that in a few years but I'm sure we will get more.

CMake indicates that this will not be a problem, but I haven't much experience to confirm their claim.

https://cmake.org/cmake/help/book/mastering-cmake/chapter/Cross%20Compiling%20With%20CMake.html#using-compile-checks

bobpaw avatar Mar 04 '25 17:03 bobpaw

@bobpaw Good find. That looks good. Lets try it.

cwsmith avatar Mar 06 '25 23:03 cwsmith