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 external PolyClipper and add dev build feature #244

Merged
merged 34 commits into from
Dec 5, 2023

Conversation

ldowen
Copy link
Collaborator

@ldowen ldowen commented Nov 1, 2023

Summary

  • This PR is a bugfix and feature PR
  • It does the following:
    • Fixes issue with using external PolyClipper
    • Fixes issue with using newer Axom version than Spack specifies
    • Updates PolyClipper and BLT versions
    • Adds options for using individual C++ shared libraries to reduce build time during development
    • Updated docs with relevant CMake variables

ToDo :

  • Annotate RELEASE_NOTES.md with notable changes.
  • Create LLNLSpheral PR pointing at this branch. (PR#63)
  • LLNLSpheral PR has passed all tests.

ldowen added 21 commits October 3, 2023 17:04
jmikeowen
jmikeowen previously approved these changes Nov 1, 2023
Copy link
Collaborator

@jmikeowen jmikeowen left a comment

Choose a reason for hiding this comment

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

These changes all look good. Once the CI tests pass should be good to merge.

if(NOT ENABLE_SHARED)
# Build static spheral C++ library
if(ENABLE_SHARED)
# Build shared spheral C++ library, currently unavailable
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this currently unavailable?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That comment is old. I will remove it.

Comment on lines 4 to 7
if(ENABLE_TESTS AND NOT ENABLE_DEV_BUILD)
blt_add_executable( NAME spheral_cuda_test
SOURCES Spheral_CUDA_Test.cc
DEPENDS_ON Spheral_CXX ${spheral_blt_cxx_depends})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we make this work with DEV_BUILD? We should be able to get the list of C++ libraries and make them a dependency right?

Copy link
Collaborator Author

@ldowen ldowen Nov 6, 2023

Choose a reason for hiding this comment

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

We might be able to reproduce what we do

if(ENABLE_DEV_BUILD)

I just wasn't sure if it was necessary since dev build mode was a specific thing that shouldn't be used in most instances.

@mdavis36 mdavis36 added this to the 2024.01.0 Release milestone Dec 4, 2023
@ldowen ldowen merged commit 39509b6 into develop Dec 5, 2023
1 check passed
@ldowen ldowen deleted the bugfix/fix_external_polyclipper branch December 5, 2023 02:10
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