Skip to content

Move some Axom Blueprint functionality into BUMP component.#1586

Merged
BradWhitlock merged 49 commits into
developfrom
feature/whitlock/axom_bump_component
Jun 10, 2025
Merged

Move some Axom Blueprint functionality into BUMP component.#1586
BradWhitlock merged 49 commits into
developfrom
feature/whitlock/axom_bump_component

Conversation

@BradWhitlock

@BradWhitlock BradWhitlock commented Jun 5, 2025

Copy link
Copy Markdown
Member

Much infrastructure for processing Blueprint meshes was developed during the course of MIR development. These features are generally useful outside of MIR so they have been refactored into a new Axom component called "BUMP" - Blueprint Utilities for Mesh Processing.

  • Blueprint-related features were moved from MIR to BUMP, updating code in MIR to compensate
  • Namespaces for some classes were shortened and code was updated
  • Some code in mir::EquizAlgorithm was moved to a details file

Additional future work (future PR) remains to consolidate Blueprint-related code (e.g. from quest) into BUMP, and possibly make BUMP more Sidre-compatible.

Comment thread src/axom/bump/doxygen_mainpage.md Outdated
Comment thread src/axom/bump/doxygen_mainpage.md Outdated
Comment thread src/axom/bump/doxygen_mainpage.md Outdated
Comment thread src/axom/bump/docs/sphinx/bump_clipping.rst Outdated

@Arlie-Capps Arlie-Capps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, Brad. I appreciate that Bump is its own component now. I appreciate your care with the docs, too.

Comment thread src/axom/CMakeLists.txt Outdated
axom_add_component(COMPONENT_NAME slam DEFAULT_STATE ${AXOM_ENABLE_ALL_COMPONENTS})
axom_add_component(COMPONENT_NAME primal DEFAULT_STATE ${AXOM_ENABLE_ALL_COMPONENTS})
axom_add_component(COMPONENT_NAME sidre DEFAULT_STATE ${AXOM_ENABLE_ALL_COMPONENTS})
axom_add_component(COMPONENT_NAME bump DEFAULT_STATE ${AXOM_ENABLE_ALL_COMPONENTS})

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@LLNL/axom General question independent of this PR: is the ordering of axom_add_component calls intended to represent strict dependencies among Axom components? If so, then I think this needs some re-assessment.

@BradWhitlock BradWhitlock Jun 9, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's what it looked like to me. Would alphabetical be better? I moved bump/mir to after spin.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Initially, I was thinking alphabetical would make it easier to keep track of, but the comment before the add component calls implies that ordering matters. We can discuss at the Axom meeting today.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it might need to be sorted by dependencies (e.g. a topological sort) since the axom_component_requires macro lists the component dependencies, e.g.

https://github.com/LLNL/axom/blob/278d1d6b4a1f398ed62f2e62c29748787e18cd5e/src/axom/multimat/CMakeLists.txt#L12-L13

Comment thread src/axom/bump/CMakeLists.txt Outdated
# Check necessary dependencies
#------------------------------------------------------------------------------
axom_component_requires(NAME BUMP
COMPONENTS SLIC SLAM PRIMAL

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@BradWhitlock follow-up to my comment above....if this line indicates the Axom dependencies for bump, then shoudln't the axom_add_component call for bump come before the axom_add_component call for sidre?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I moved bump/mir to after spin in that list of axom_add_component calls.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Aaaand it looks like I need to add spin there.

@kennyweiss kennyweiss left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @BradWhitlock -- this is going to be a really useful component for blueprint-related workflows.

Please don't forget to update the RELEASE_NOTES before merging.

Comment thread src/axom/bump/clipping/ClipField.hpp Outdated
Comment thread src/axom/bump/clipping/ClipTableManager.hpp Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great overview!

Comment thread src/axom/bump/tests/bump_utilities.cpp
Comment thread src/axom/bump/utilities/conduit_memory.hpp Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it make sense to move these conversion types up to sidre?

(They're ok here too).

Comment thread src/axom/bump/utilities/utilities.hpp Outdated
Comment thread src/axom/bump/utilities/utilities.hpp Outdated
Comment thread src/axom/bump/utilities/utilities.hpp Outdated

@rhornung67 rhornung67 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you @BradWhitlock

@BradWhitlock
BradWhitlock merged commit 1258aef into develop Jun 10, 2025
15 checks passed
@BradWhitlock
BradWhitlock deleted the feature/whitlock/axom_bump_component branch June 10, 2025 23:24
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.

4 participants