Skip to content

Conversation

@hmenke
Copy link
Member

@hmenke hmenke commented Oct 3, 2025

@the-hampel
Copy link
Member

Hi @hmenke ,

thank you for changing this to the recommended f2py setup. Commits looks good to me. One question: do you know if this workaround I added for the intel compiler:

if(CMAKE_Fortran_COMPILER_ID MATCHES "Intel")
  set(F2PY_INTEL_OPTIONS -liomp5 --fcompiler=intelem)
endif()

is no longer necessary for f2py? The Jenkins CI for intel ubuntu is passing so I assume it is fine. But maybe you know something more concrete?

Best,
Alex

@hmenke
Copy link
Member Author

hmenke commented Oct 3, 2025

getpmatelk doesn't really use OpenMP. There is a critical section around the IO part, but getpmatelk is never called from within an OpenMP loop.

Even if OpenMP is really desired for some reason, the correct way to use it with my proposed changes would be the below diff. This will then also work correctly on any compiler.

--- a/python/triqs_dft_tools/converters/elktools/elkwrappers/CMakeLists.txt
+++ b/python/triqs_dft_tools/converters/elktools/elkwrappers/CMakeLists.txt
@@ -26,6 +26,9 @@ Python_add_library(getpmatelk MODULE
 set_property(TARGET getpmatelk PROPERTY SUFFIX "${TRIQS_PYTHON_MODULE_EXT}")
 target_link_libraries(getpmatelk PRIVATE fortranobject)
 
+find_package(OpenMP COMPONENTS Fortran)
+target_link_libraries(getpmatelk PRIVATE OpenMP::OpenMP_Fortran)
+
 install(TARGETS getpmatelk DESTINATION ${TRIQS_PYTHON_LIB_DEST_ROOT}/${PROJECT_NAME}/converters/elktools/elkwrappers)
 
 # user warning

@the-hampel
Copy link
Member

Okay I see. I am also not sure why I added this weird workaround back then. Let me merge it like this and if any problem comes up at any point I will remember now. Thanks!

@the-hampel the-hampel merged commit febd0ad into TRIQS:unstable Oct 3, 2025
5 checks passed
@hmenke hmenke deleted the f2py branch October 3, 2025 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants