Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix building shared libraries on macOS with --enable-libtorch #1123

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

Luthaf
Copy link
Contributor

@Luthaf Luthaf commented Oct 8, 2024

Description

The configure code was unconditionally adding -Wl;--no-as-needed to the flags, even though these are not valid on macOS.

I'm not sure why this needs --no-as-needed on linux on the first place, @luigibonati do you remember why this is the case?

Type of contribution
  • changes to code or doc authored by PLUMED developers, or additions of code in the core or within the default modules
  • changes to a module not authored by you
  • new module contribution or edit of a module authored by you
Copyright
  • I agree to transfer the copyright of the code I have written to the PLUMED developers or to the author of the code I am modifying.
  • the module I added or modified contains a COPYRIGHT file with the correct license information. Code should be released under an open source license. I also used the command cd src && ./header.sh mymodulename in order to make sure the headers of the module are correct.
Tests
  • I added a new regtest or modified an existing regtest to validate my changes.
  • I verified that all regtests are passed successfully on GitHub Actions.

@Luthaf
Copy link
Contributor Author

Luthaf commented Oct 8, 2024

The CI errors looks very strange: they seem to be about different whitespaces in parsing output:

#11 663.2 /home/plumed/plumed2/regtest/contour/rt-parse-only/tmp
#11 663.2 Checking for contour
#11 663.5 FAILURE
#11 663.5 Diff for shortcuts.json:
#11 663.5 16c16
#11 663.5 <       "expansion" : "fcc_grp: GROUP ATOMS=1-5184 \nfcc_mat: CONTACT_MATRIX GROUP=1-5184 SWITCH={CUBIC D_0=1.2 D_MAX=1.5} COMPONENTS \nfcc_vfunc: FCCUBIC_FUNC ARG=fcc_mat.x,fcc_mat.y,fcc_mat.z ALPHA=27\nfcc_wvfunc: CUSTOM ARG=fcc_vfunc,fcc_mat.w FUNC=x*y PERIODIC=NO\nfcc_ones: ONES SIZE=5184\nfcc: MATRIX_VECTOR_PRODUCT ARG=fcc_wvfunc,fcc_ones\nfcc_denom: MATRIX_VECTOR_PRODUCT ARG=fcc_mat.w,fcc_ones\nfcc_n: CUSTOM ARG=fcc,fcc_denom FUNC=x/y PERIODIC=NO"
#11 663.5 ---
#11 663.5 >       "expansion" : "fcc_grp: GROUP ATOMS=1-5184\nfcc_mat: CONTACT_MATRIX GROUP=1-5184 SWITCH={CUBIC D_0=1.2 D_MAX=1.5} COMPONENTS\nfcc_vfunc: FCCUBIC_FUNC ARG=fcc_mat.x,fcc_mat.y,fcc_mat.z ALPHA=27\nfcc_wvfunc: CUSTOM ARG=fcc_vfunc,fcc_mat.w FUNC=x*y PERIODIC=NO\nfcc_ones: ONES SIZE=5184\nfcc: MATRIX_VECTOR_PRODUCT ARG=fcc_wvfunc,fcc_ones\nfcc_denom: MATRIX_VECTOR_PRODUCT ARG=fcc_mat.w,fcc_ones\nfcc_n: CUSTOM ARG=fcc,fcc_denom FUNC=x/y PERIODIC=NO"

which I don't see how these changes could have introduced??

@GiovanniBussi
Copy link
Member

@gtribello do you know the origin of this failure? It seems related to the parser

@carlocamilloni
Copy link
Member

@Luthaf can you retarget to v2.10 please?

@Luthaf Luthaf changed the base branch from master to v2.10 October 10, 2024 15:16
@Luthaf
Copy link
Contributor Author

Luthaf commented Oct 10, 2024

Sure, I'll rebase as well to remove the extra commits.

I'll need this and a couple of other things for my work, should I also backport #1122 to v2.10?

The configure code was unconditionally adding `-Wl;--no-as-needed` to the flags, even though
these are not valid on macOS.
@Luthaf Luthaf force-pushed the build-dylib-macos-with-torch branch from cc47aab to 74ca55f Compare October 10, 2024 15:18
@carlocamilloni
Copy link
Member

carlocamilloni commented Oct 10, 2024

yes for the moment everything that can be considered bug fixing should target v2.10 or v2.9

@GiovanniBussi
Copy link
Member

Regarding #1122, I just backported it to v2.10

@carlocamilloni carlocamilloni merged commit d72cc4d into plumed:v2.10 Oct 10, 2024
16 of 19 checks passed
@Luthaf Luthaf deleted the build-dylib-macos-with-torch branch October 10, 2024 16:32
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.

3 participants