-
Notifications
You must be signed in to change notification settings - Fork 27
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
Adds opencascade as an optional dependency #1441
base: develop
Are you sure you want to change the base?
Conversation
f5c930e
to
ba9bf06
Compare
/style |
Only currently tested on LLNL's toss4.
Misc: Fixes typo in comments about exported targets
Includes a test for reading and writing STEP files.
I am not sure why this has not shown up previously.
I had to copy the opencascade port files from vcpkg to * remove the default-dependency on freetype * patch a typo where a number was being quoted in a CMake file
Patch is from A. Dayton and is slightly past raja@2024.07.0
…ed with a query point
The candidates are currently returned as a std::vector. Misc: Uses locales to improve output of these tests.
27552e8
to
4d1304e
Compare
set(UMPIRE_DIR "${TPL_ROOT}/umpire-2024.07.0-4bdhkgxmckkcfnhfv76gutbzn7gzrcum" CACHE PATH "") | ||
set(UMPIRE_DIR "${TPL_ROOT}/umpire-2024.07.0-et2uff5thiprgch43uftatndduzssxay" CACHE PATH "") | ||
|
||
set(OPENCASCADE_DIR "${TPL_ROOT}/opencascade-7.8.1-epyyoyevoozkzqoxdqxku2q4anwlqw52" CACHE PATH "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested configs on all platforms w/ opencascade
, but only enabled it on a single toss4 config on LC.
I also enabled opencascade in Windows configs.
buildable: False | ||
externals: | ||
- spec: mesa@18.3.4 # found via: glxinfo | grep -i version | ||
prefix: / |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@white238 -- the opengl and X
libraries were in /lib64
on toss4_cray
and blueos
, so I made the prefix /
instead of /usr
. Is that the correct prefix? I tested this w/ one of our blueos configs and it worked.
### BEGIN AXOM PATCH | ||
|
||
# replace a quoted number in a flag -- /wd"26812" with /wd26812 | ||
set(_file "${SOURCE_PATH}/adm/cmake/occt_defs_flags.cmake") | ||
file(READ "${_file}" _filedata) | ||
string(REPLACE [[/wd\"26812\"]] [[/wd26812]] _filedata "${_filedata}") | ||
file(WRITE "${_file}" "${_filedata}") | ||
### END AXOM PATCH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: I had to copy the whole opencascade
portfile to add this patch, which converts /wd"26812"
to /wd26812
"host": true | ||
} | ||
], | ||
"default-features": [], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: I copied the opencascade
portfile from vcpkg
to remove the freetype
default-feature.
More direct ways to do this would involve updates to uberenv to have a project package file.
REF 85c58eb7d25ac5a7ff1c6a3cc6bb74bf8351e36e # HEAD of bugfix/dayton8/win32 branch; a bit past v2024.07.0 | ||
SHA512 263346ff91c17f48fa40efdd6826331e1fecb995fef289bf997500d058c57e4dc706512baa8fc5dfc912c54939b973c56c8677a4705e261a2740c55851446341 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: This points to @adayton1's bugfix commit -- LLNL/RAJA#1746
I can update it to the current HEAD of raja@develop now that the PR is merged (or leave it as is).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm OK with either. If users need this, I can tag a patch release for RAJA because Mike C. won't build any TPL sets that don't point at releases.
Please let me know if I should do a RAJA release.
@@ -351,6 +352,11 @@ class PointFinder | |||
return m_cellBBoxes[cellIdx]; | |||
} | |||
|
|||
std::vector<IndexType> getCandidates(SpacePoint const& pt) const | |||
{ | |||
return m_grid.getCandidatesAsArray(pt); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: ImplicitGrid
returns a std::vector
for its getCandidatesAsArray
. We should probably change this to an axom::Array
so it's amenable w/ usage on a device.
#if(defined(__x86_64__) && defined(__GNUC__)) || \ | ||
(defined(_WIN64) && (_MSC_VER >= 1600)) | ||
using Word = std::uint64_t; | ||
#else | ||
using Word = std::uint32_t; | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: BitSet
now uses 32-bit words in x86
mode on windows.
const int numBits = | ||
axom::utilities::min(bitsPerWord, nbits - (iword * bitsPerWord)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: I lost way too much time trying to track down this bug! It hardcoded 64-bit words for the underlying slam::BitSet
!
Thank you for this work, @kennyweiss . So far it looks good. What docs do you plan to add? |
Thanks @agcapps -- I am planning to add a line to mention opencascade as an optional dependency and mention how to set it up w/ the |
Summary
TODO