Skip to content

Fix a typo in the tmLQCD cmake module#694

Merged
kostrzewa merged 2 commits into
masterfrom
cmake_fix
May 29, 2026
Merged

Fix a typo in the tmLQCD cmake module#694
kostrzewa merged 2 commits into
masterfrom
cmake_fix

Conversation

@mtaillefumier

@mtaillefumier mtaillefumier commented May 28, 2026

Copy link
Copy Markdown
Contributor

The tmLQCD module was not working as intended because of some typo and absence of tmLQCD cmake modules path. This PR solves these issues.

  • lib_wrapper.c is compiled and included in the library. The wrapper code can be found in src/lib/wrapper.c
  • Removed a forgotten Makefile.in
  • Removed a wrong comment

Comment thread cmake/tmLQCDConfig.cmake.in Outdated

set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/cmake" ${CMAKE_MODULE_PATH})

# store CXX compiler id. Used in MKL package.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is TM_C_COMPILER_ID really used anywhere?

@mtaillefumier

Copy link
Copy Markdown
Contributor Author

Is it better to have lib_wrapper separated or included ?

@mtaillefumier mtaillefumier requested a review from kostrzewa May 29, 2026 09:42
@kostrzewa

Copy link
Copy Markdown
Member

It's better to have it included I think.

As you can see here:

https://github.com/marcuspetschlies/cvc/blob/dev-cpff/Makefile_examples/Makefile.daint.cuda (line 13)

because tmLQCD is so poorly engineered one had to link against all of the sub libs anyway. Might as well link against -ltmlqcd and be done with it.

I don't believe that having the functions in lib_wrapper available in tmLQCD itself will harm anyone.

Comment thread cmake/tmLQCDConfig.cmake.in Outdated
# store CXX compiler id. Used in MKL package.
set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/cmake" ${CMAKE_MODULE_PATH})

set(TM_C_COMPILER_ID @CMAKE_C_COMPILER_ID@)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mtaillefumier what I was referring to was that as far as I can tell it's not just the comment which can be removed but all the lines referring to TM_C_COMPILER_ID no? Maybe I'm missing something but I couldn't find this being used elsewhere.

@mtaillefumier

mtaillefumier commented May 29, 2026 via email

Copy link
Copy Markdown
Contributor Author

@kostrzewa

Copy link
Copy Markdown
Member

Thanks for confirming. The current pr included it in the library so unless something else is missing this pr should be good for merging after review. Sent from Proton Mail for Android.

-------- Original Message --------
On Friday, 05/29/26 at 11:50 Bartosz Kostrzewa @.> wrote: kostrzewa left a comment [(etmc/tmLQCD#694)](#694 (comment)) It's better to have it included I think. As you can see here: https://github.com/marcuspetschlies/cvc/blob/dev-cpff/Makefile_examples/Makefile.daint.cuda (line 13) because tmLQCD is so poorly engineered one had to link against all of the sub libs anyway. Might as well link against -ltmlqcd and be done with it. I don't believe that having the functions in lib_wrapper available in tmLQCD itself will harm anyone. — Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today! You are receiving this because you authored the thread.Message ID: @.>

It would be great if you could take a look at the TM_C_COMPILER_ID thing above. I don't see that this is used any where but maybe I'm missing something.

@mtaillefumier

Copy link
Copy Markdown
Contributor Author

I can not find any useful case for it either so I removed it completely.

@kostrzewa kostrzewa merged commit 0957a87 into master May 29, 2026
3 checks passed
@kostrzewa

kostrzewa commented May 29, 2026

Copy link
Copy Markdown
Member

Thanks @mtaillefumier

@jing2li you should now be able to link against tmLQCD again from CVC passing just -ltmlqcd instead of the long list of libraries

@kostrzewa kostrzewa deleted the cmake_fix branch May 29, 2026 14:05
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