Skip to content

Avoid overflow for volume fraction calculation - #1895

Open
BradWhitlock wants to merge 11 commits into
developfrom
bugfix/whitlock/batched_volfrac_creation_mfem
Open

Avoid overflow for volume fraction calculation#1895
BradWhitlock wants to merge 11 commits into
developfrom
bugfix/whitlock/batched_volfrac_creation_mfem

Conversation

@BradWhitlock

@BradWhitlock BradWhitlock commented Jun 26, 2026

Copy link
Copy Markdown
Member

This is a follow-on for #1875 (merged into develop now).

This PR resolves #1871 by:

  • Changing MFEM shaping helper shaping::computeVolumeFractionsForMaterial so it works on groups of elements rather than all elements at once when it looks like the number of values will overflow int32
  • Passes execution policy to shaping::computeVolumeFractionsForMaterial so we can parallelize for OpenMP some of the more expensive algorithm phases

Shaping driver varying mesh size for spheres.yaml input with order 4 output
image

Base automatically changed from feature/whitlock/sampling_low_order_blueprint_mesh to develop August 26, 2026 18:36

@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

It's a bit difficult to tell from the github review page, but it looks like the shaping_helpers_mfem no longer runs on device, where previously several parts (but not all of it) ran on device, when using a hip or cuda policy.
Can you please run this in a GPU config/policy and confirm that we're not losing functionality? The speedups were significant in some of my runs, and getting the memory transfers correct/working was challenging.

Update: I don't think any of our hip/cuda configs are currently using mfem::Device functionality. Outside of this PR, we should enable/exercise mfem on-device for our shaping problems in our hip/cuda configs.
I created this issue to track this: #1966

* \param volfracOrder The order of the volume fraction function to create.
* \param sampleResolution The number of samples in each mesh dimension.
* \param quadratureType The quadrature type that determines the sample point locations.
*/

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.

Minor: Please update the doxygen for this function

axom::ArrayView<int> sampleResolution,
axom::numerics::QuadratureType quadratureType)
{
constexpr std::int64_t MAX_CACHED_MASS_BYTES = 1LL << 30;

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.

Please add a comment to this line.
Presumably, it is sized to avoid overflow w/ int32.

Should we upstream a concern/fix to mfem about overflow w.r.t. int32 vs int64 indices?

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.

Perhaps it would be sufficient (or at least an improvement) for mfem to check for and give a warning about overflow?

Comment on lines +74 to +77
SLIC_WARNING_ROOT(
axom::fmt::format("MFEM volume fraction processing for '{}' uses host data and currently "
"falls back to sequential execution for device runtime policies.",
vfName));

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.

Should we warn the user about this?
Our implementation (in Axom/MFEM) is only on the CPU, so there's nothing they can do about it.

If we do want to post a warning, perhaps it would be sufficient to only warn once in a given run?
E.g. using SLIC_WARNING_ROOT_ONCE

/*!
* \def SLIC_WARNING_ROOT_ONCE( msg )
* \brief Macro that logs given warning message only on root, and only once per call site.
*
* \param [in] msg user-supplied message.
*
* \note The SLIC_WARNING_ROOT_ONCE macro is always active.
* \note By default, all ranks are considered to be root.
* Must call `axom::slic::initialize(is_root={true|false})`
* or set via `axom::slic::setIsRoot({true|false})` to filter based on root.
*
* Usage:
* \code
* SLIC_WARNING_ROOT_ONCE( "A warning has occurred!" );
* \endcode
*
*/
#define SLIC_WARNING_ROOT_ONCE(msg) \
SLIC_DETAIL_LOG_IF_ONCE(SLIC_WARNING_IF, axom::slic::isRoot(), msg)

applyFCTProjectionImpl<omp_exec>(massMat, rhs, dofs, numElements, vfData, scratchBuffer);
break;
#endif
default:

@kennyweiss kennyweiss Aug 26, 2026

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'm reasonably sure that the FCT ran on device.

Update: Nevermind -- this never ran on device

AXOM_ANNOTATE_BEGIN("fct project");
axom::for_all<axom::SEQ_EXEC>(0, NE, [=](int i) {
FCT_correct(&m_d(0, 0, i), dofs, &b_d(0, i), minY, maxY, &vf_d(0, i), &fct_mat_d(0, 0, i));
});
AXOM_ANNOTATE_END("fct project");
}

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.

int32 overflow leads to crash in shaping_driver.

2 participants