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

Add Celeritas cmake command wrappers for downstream code #1678

Merged
merged 7 commits into from
Mar 18, 2025

Conversation

sethrj
Copy link
Member

@sethrj sethrj commented Mar 18, 2025

This follow-on to #1673 restores and automatically exposes celeritas_-prefixed wrappers for downstream applications:

  • This allows us to someday not distribute CudaRdcUtils if it's not applicable, or if the capabilities get built into cmake
  • It makes it more obvious to downstream users where the command is coming from
  • It improves configure performance if CUDA or VecGeom are disabled

@sethrj sethrj added enhancement New feature or request core Software engineering infrastructure labels Mar 18, 2025
@sethrj sethrj requested a review from pcanal March 18, 2025 16:39
Copy link

github-actions bot commented Mar 18, 2025

Test summary

 4 291 files   6 548 suites   13m 38s ⏱️
 1 782 tests  1 771 ✅ 11 💤 0 ❌
22 842 runs  22 759 ✅ 83 💤 0 ❌

Results for commit dfdd793.

♻️ This comment has been updated with latest results.

@sethrj sethrj marked this pull request as ready for review March 18, 2025 18:36
@sethrj sethrj force-pushed the celeritas-wrappers branch from 30f06e9 to 3b201c7 Compare March 18, 2025 18:36
@sethrj sethrj requested a review from stognini March 18, 2025 20:44
@stognini
Copy link
Member

Since cuda_rdc_target_link_libraries was refactored to celeritas_target_link_libraries, is the include(CudaRdcUtils) going to be updated to something like include(CeleritasUtils)?

@sethrj
Copy link
Member Author

sethrj commented Mar 18, 2025

Since cuda_rdc_target_link_libraries was refactored to celeritas_target_link_libraries, is the include(CudaRdcUtils) going to be updated to something like include(CeleritasUtils)?

cuda_rdc_target_link_libraries still exists, but now it's preferred for downstream libraries to call celeritas_target_link_libraries for the reasons describe in the header. To simplify things for users, and to be more like other libraries (e.g., Clang and ROOT both provide helper macros without you having to include a separate file) users will no longer have to include anything.

@stognini
Copy link
Member

e.g., Clang and ROOT both provide helper macros without you having to include a separate file

Oh I see, that is what you meant when you mentioned not distribute CudaRdcUtils if it's not applicable. Thanks

@sethrj sethrj force-pushed the celeritas-wrappers branch from 4a558a9 to c11cf19 Compare March 18, 2025 21:12
@sethrj sethrj force-pushed the celeritas-wrappers branch from e056d0e to 8223b55 Compare March 18, 2025 22:10
@sethrj
Copy link
Member Author

sethrj commented Mar 18, 2025

@esseivaju I'm going to add another quick follow-on to fix the perfetto installation. @pcanal could you give this a quick lookover?

This reverts commit 737a2d9.

I guess we do have to export them for static builds...
Copy link
Contributor

@pcanal pcanal left a comment

Choose a reason for hiding this comment

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

LGTM.

@sethrj sethrj merged commit 9e0aef4 into celeritas-project:develop Mar 18, 2025
38 checks passed
sethrj added a commit to sethrj/celeritas that referenced this pull request Mar 22, 2025
…roject#1678)

* Reapply "Move and always install celeritas library wrappers"
* Fix MPI build
* Fix typo, use more foolproof way
* We *do* need to export object libraries
* Fix copy-paste error
sethrj added a commit that referenced this pull request Mar 22, 2025
* Reapply "Move and always install celeritas library wrappers"
* Fix MPI build
* Fix typo, use more foolproof way
* We *do* need to export object libraries
* Fix copy-paste error
sethrj added a commit to sethrj/celeritas that referenced this pull request Mar 22, 2025
…roject#1678)

* Reapply "Move and always install celeritas library wrappers"
* Fix MPI build
* Fix typo, use more foolproof way
* We *do* need to export object libraries
* Fix copy-paste error
sethrj added a commit to sethrj/celeritas that referenced this pull request Mar 22, 2025
…roject#1678)

* Reapply "Move and always install celeritas library wrappers"
* Fix MPI build
* Fix typo, use more foolproof way
* We *do* need to export object libraries
* Fix copy-paste error
sethrj added a commit to sethrj/celeritas that referenced this pull request Mar 22, 2025
…roject#1678)

* Reapply "Move and always install celeritas library wrappers"
* Fix MPI build
* Fix typo, use more foolproof way
* We *do* need to export object libraries
* Fix copy-paste error
@sethrj sethrj added minor Minor internal changes or fixes and removed enhancement New feature or request labels Mar 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Software engineering infrastructure minor Minor internal changes or fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants