Skip to content

Conversation

@wavefunction91
Copy link
Owner

@wavefunction91 wavefunction91 commented Mar 26, 2024

Previously, the GAUXC_ENABLE_XYZ variables did all the heavy lifting for feature detection and control. Per @loriab's comments and subsequent issue (#102),these variables sound suspiciously editable, so probably best to not rely on that naming scheme.

This PR separates out GAUXC_ENABLE_XYZ to trigger detection of feature XYZ and sets GAUXC_HAS_XYZ if the feature has been detected and enabled throughout the stack.

Major Change: Do not use GAUXC_ENABLE_XYZ downstream to check feature behaviour, only use these variables to enable features via FetchContent and use GAUXC_HAS_XYZ to check if the feature enable happened. The latter should also be used for source-level feature detection (e.g. GAUXC_HAS_DEVICE can be used to check if GauXC has device bindings)

Closes #102

TODO: Check downstream integrations:

  • NWChemEx
  • MPQC
  • ExaChem
  • ChronusQ
  • Psi4

@wavefunction91
Copy link
Owner Author

Ping @evaleev @dmejiar @ajaypanyala @elambros @loriab @davpoolechem

This will undoubtedly break some things in some build systems. Please check that you can make this work in your respective code bases. I won't merge until we can get consensus that we're not breaking things downstream.

@ajaypanyala
Copy link
Contributor

ExaChem is good to go!

@wavefunction91
Copy link
Owner Author

@davpoolechem
Copy link
Contributor

Can confirm that this works for Psi4!

@wavefunction91
Copy link
Owner Author

NWChemEx integration is fixed

@wavefunction91
Copy link
Owner Author

@elambros can you confirm that this PR works in CQ? It will require changing all instances of GAUXC_ENABLE_DEVICE to GAUXC_HAS_DEVICE for the device feature check in the code (fetch/configure should stay the same)

@elambros
Copy link
Contributor

Yep works on the CQ end too!

@wavefunction91 wavefunction91 merged commit ecfeb5f into master Mar 28, 2024
@wavefunction91 wavefunction91 deleted the issue_102/fix_cmake_var_naming branch March 28, 2024 19:16
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.

CMake Variable Naming Concerns

5 participants