Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
e84da5d
Add equidistribution marking (c++)
schnellerhase Apr 24, 2026
6945e6f
Pull out common routine mark_threshold
schnellerhase Apr 27, 2026
c4a2bdb
Add equidistribution marking (py)
schnellerhase Apr 27, 2026
a5ff0dd
Merge branch 'main' into schnellerhase/equidistribution-marking
schnellerhase Apr 30, 2026
e997738
Use inner prodcut
schnellerhase Apr 30, 2026
b9831ba
Improve docstring
schnellerhase Apr 30, 2026
6e9b9c4
use latex
schnellerhase Apr 30, 2026
59af09c
Merge branch 'main' into schnellerhase/equidistribution-marking
schnellerhase May 1, 2026
de0b6a0
Docs: differentiate between eta and marker consistently
schnellerhase May 1, 2026
96ae33e
Merge branch 'main' into schnellerhase/equidistribution-marking
schnellerhase May 4, 2026
4a63e73
Merge branch 'main' into schnellerhase/equidistribution-marking
schnellerhase May 5, 2026
b4690b8
Merge branch 'main' into schnellerhase/equidistribution-marking
schnellerhase May 6, 2026
573e125
Merge branch 'main' into schnellerhase/equidistribution-marking
schnellerhase May 7, 2026
10575df
Merge branch 'main' into schnellerhase/equidistribution-marking
schnellerhase May 7, 2026
77d35fe
Add mark_equidistribution_squared
schnellerhase May 7, 2026
425a642
Fix and python exports
schnellerhase May 7, 2026
63ace3f
Drop code duplication
schnellerhase May 7, 2026
8d2ae29
format
schnellerhase May 7, 2026
c588fb7
Rename eta to indicator
schnellerhase May 7, 2026
f56fae3
format
schnellerhase May 7, 2026
b768685
Apply suggestions from code review
schnellerhase May 7, 2026
3d3be2a
Merge branch 'main' into schnellerhase/equidistribution-marking
schnellerhase May 8, 2026
aa60ca1
Merge branch 'main' into schnellerhase/equidistribution-marking
schnellerhase May 11, 2026
d3b4a9b
Explicit capture
schnellerhase May 11, 2026
73ea789
Improve docs
schnellerhase May 11, 2026
10ced01
Merge branch 'main' into schnellerhase/equidistribution-marking
schnellerhase May 11, 2026
6b2cca2
Apply docstring improvements
schnellerhase May 12, 2026
625957b
Remove walrus
schnellerhase May 12, 2026
d9656db
Pass dtype to random
schnellerhase May 12, 2026
cf0121f
Comm first arg
schnellerhase May 12, 2026
9b48e05
Merge branch 'main' into schnellerhase/equidistribution-marking
schnellerhase May 14, 2026
8dd7c9f
Merge branch 'main' into schnellerhase/equidistribution-marking
schnellerhase May 15, 2026
a65c3c1
WIP - Docstring adjustments
jhale May 19, 2026
08bb596
fixup docs
schnellerhase May 19, 2026
cb56b7a
Merge branch 'main' into schnellerhase/equidistribution-marking
schnellerhase May 19, 2026
6f3733c
Fixup docs and remove potential overflow on summing std::int32_t
jhale May 20, 2026
477c05c
size_t
jhale May 20, 2026
a498c3b
This is optimal unless the expected mark fraction is very small (< 5%).
jhale May 20, 2026
e533aed
Fixes
jhale May 20, 2026
5fd1a96
More fixes
jhale May 20, 2026
5455f00
Fix wrappers nbarg and consistent variable names
jhale May 20, 2026
f17e463
clang-format
jhale May 20, 2026
ddc3eb4
markers -> indicators in tests
jhale May 20, 2026
15e0438
Improve docstring
jhale May 20, 2026
fa679a2
Add Python wrappers for mark_* refinement functions
jhale May 20, 2026
c475ca0
Use Real
jhale May 20, 2026
43b5203
markers -> indicators in mark.cpp test
jhale May 20, 2026
ffc96ab
Make squared_indicators very explicit
jhale May 20, 2026
5dbcea4
Fix up
jhale May 20, 2026
86f9765
Ruff manual fix
jhale May 20, 2026
4936a94
Raw strings
jhale May 20, 2026
254c711
Fix double backslashes in raw string docstrings
jhale May 20, 2026
7c5f5a6
Remove std::pow
jhale May 20, 2026
6caed8d
Add warning to docstring
jhale May 20, 2026
bcc5bcd
Add to Python docstring
jhale May 20, 2026
3b464e1
Also fix up tests to show correct usage.
jhale May 20, 2026
573b8da
Simplify docstrings
jhale May 20, 2026
e5e0564
Revert to count_if
jhale May 20, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 64 additions & 9 deletions cpp/dolfinx/refinement/mark.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,34 @@
namespace dolfinx::refinement
{

namespace impl
Comment thread
jhale marked this conversation as resolved.
{

/// @brief Threshold marking helper
Comment thread
schnellerhase marked this conversation as resolved.
Outdated
///
/// @param[in] marker Input marker
/// @param[in] threshold Lower bound for values to mark
Comment thread
schnellerhase marked this conversation as resolved.
Outdated
///
/// @returns indices i which satisfy e_i > threshold.
template <std::floating_point T>
std::vector<std::int32_t> mark_threshold(std::span<const T> marker, T threshold)
{
auto mark = [=](T e) { return e > threshold; };
Comment thread
schnellerhase marked this conversation as resolved.
Outdated

std::vector<std::int32_t> indices;
indices.reserve(std::ranges::count_if(marker, mark));

for (std::int32_t i = 0; i < static_cast<std::int32_t>(marker.size()); ++i)
{
if (mark(marker[i]))
indices.push_back(i);
}

return indices;
}

} // namespace impl

/// @brief Maximum marking of a marker.
Comment thread
schnellerhase marked this conversation as resolved.
Outdated
///
/// @param[in] marker Input marker (local) - usually an error indicator per
Expand All @@ -39,18 +67,45 @@ std::vector<std::int32_t> mark_maximum(std::span<const T> marker, T theta,
: std::ranges::max(marker);
MPI_Allreduce(MPI_IN_PLACE, &max, 1, dolfinx::MPI::mpi_t<T>, MPI_MAX, comm);

auto mark = [=](T e) { return e > theta * max; };
auto indices = impl::mark_threshold<T>(marker, theta * max);

std::vector<std::int32_t> indices;
indices.reserve(std::ranges::count_if(marker, mark));
spdlog::info("Marking (max) {} / {} (local) entities.", indices.size(),
Comment thread
schnellerhase marked this conversation as resolved.
Outdated
marker.size());

for (std::int32_t i = 0; i < static_cast<std::int32_t>(marker.size()); ++i)
{
if (mark(marker[i]))
indices.push_back(i);
}
return indices;
}

spdlog::info("Marking (max) {} / {} (local) entities.", indices.size(),
/// @brief Equidistribution marking of a marker.
Comment thread
schnellerhase marked this conversation as resolved.
Outdated
///
Comment thread
schnellerhase marked this conversation as resolved.
/// @param[in] marker Input marker (local) - usually an error indicator per
Comment thread
schnellerhase marked this conversation as resolved.
Outdated
/// entity
/// @param[in] theta Parameter, 0 < θ < 1
/// @param[in] comm Communicator over which the total marker is computed.
Comment thread
schnellerhase marked this conversation as resolved.
Outdated
/// @return Indices (local) of marker elements, which satisfy: marker_i ≥ θ
Comment thread
schnellerhase marked this conversation as resolved.
Outdated
/// (sum_i marker_i^2)^1/2 / N^1/2.

@jhale jhale Apr 30, 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.

Please correct me if I'm wrong, but there is probably no need to do so many square root operations - assuming markers/indicators and theta are always positive, you can square both sides:

marker_i^2 ≥ θ^2 (sum_i marker_i^2) / N.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's true but we would sacrifice the shared code path of mark_threshold. Since it's 2x sqrt's not critical?

@jhale jhale Apr 30, 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.

Not really - in the vast majority of cases I'm familiar with, one naturally assembles the square of the entity estimator, so your approach effectively requires the user to do many square roots outside this function.

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.

See e.g. https://doi.org/10.1016/j.camwa.2022.11.009 eq A.1, A.4, 3.15 (which should really be squared on both sides).

@schnellerhase schnellerhase May 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That would require changing the input to be interpreted as squared already which complicates the interface: the equidistribution property is based on $\eta$ (not $\eta^2$) so invoking with $\eta^2$ and equilibrating wrt. to $\eta$ is easy to mess up.

If one wants to avoid the sqrt's (per entity) on caller side for performance optimisations, one can replace mark_equidistribution(marker, theta, ...) with mark_equidistribution(marker/sqrt(N), theta, ...)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree, but they are valid indicators for selecting a region of interest. That's why we should not have an interface which requires squared quantities as input, right?

@jhale jhale May 2, 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.

Then why are you squaring them on the inside? Both my design and your design imply the suitability of the l^2 norm - in the proper AFEM setting this suitability is entirely justified. For your 'indicators' it is a completely unjustified imposition.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That would be a users responsibility to ensure that this checks out (if there is any theory on that). The maximum marking is also not assuming an underlying l2 structure and gets heavily used in AFEM schemes (for noisy indicators, I think). The interface should not assume structure on the input data. Any other norm based, or not norm based, measure could be supported.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What if we added something like a marker_is_squared flag? While this would have no effect for maximum marking or Dörfler marking, where it is the user’s responsibility to provide either squared or non-squared markers, as well as the correct marking parameter, this could solve the issue with the equidistribution strategy.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I like the idea - feels like a practical resolution to the problem honouring that the l2 case is the 95% use case and allowing for general interface usage in the meantime. Only I would prefer to provide the additional argument to all functions if we enable it (so also for max marking for now), for a consistent user interface - even though it is not strictly functionally required. What do you think @jhale?

template <std::floating_point T>
std::vector<std::int32_t> mark_equidistribution(std::span<const T> marker,
T theta, MPI_Comm comm)
{
if ((theta <= 0) || (theta >= 1))
throw std::invalid_argument("Theta needs to fullfill 0 < θ < 1.");

T norm{0};
Comment thread
schnellerhase marked this conversation as resolved.
Outdated
for (T e : marker)
norm += std::pow(e, 2);

MPI_Allreduce(MPI_IN_PLACE, &norm, 1, dolfinx::MPI::mpi_t<T>, MPI_SUM, comm);

norm = std::sqrt(norm);

std::int32_t count = marker.size();
MPI_Allreduce(MPI_IN_PLACE, &count, 1, dolfinx::MPI::mpi_t<std::int32_t>,
MPI_SUM, comm);

auto indices
= impl::mark_threshold<T>(marker, theta * norm / std::sqrt(count));

spdlog::info("Marking (equi) {} / {} (local) entities.", indices.size(),
Comment thread
schnellerhase marked this conversation as resolved.
Outdated
marker.size());

return indices;
Expand Down
41 changes: 41 additions & 0 deletions cpp/test/mesh/refinement/mark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,44 @@ TEMPLATE_TEST_CASE("Mark maximum", "[refinement][mark][maximum]", double, float)
CHECK(expect_marked == marked);
}
}

TEMPLATE_TEST_CASE("Mark equidistribution empty",
"[refinement][mark][equidistribution]", double, float)
{
std::vector<TestType> marker;
auto indices = mark_equidistribution<TestType>(marker, .5, MPI_COMM_WORLD);
CHECK(indices.size() == 0);
}

TEMPLATE_TEST_CASE("Mark equidistribution",
Comment thread
jhale marked this conversation as resolved.
"[refinement][mark][equidistribution]", double, float)
{
MPI_Comm comm = MPI_COMM_WORLD;

std::vector<TestType> marker;
marker.reserve(10);
for (std::size_t i = 0; i < 10; i++)
marker.push_back(10 * dolfinx::MPI::rank(comm) + i);

TestType theta = 0.5;
auto indices = mark_equidistribution<TestType>(marker, theta, comm);

CHECK(std::ranges::all_of(
indices, [&](auto e)
{ return (0 <= e) && (e <= static_cast<std::int32_t>(marker.size())); }));

std::int32_t n = dolfinx::MPI::size(comm) * 10 - 1;
TestType norm = std::sqrt(n * (n + 1) * (2 * n + 1) / 6);

auto mark = [=](auto e) { return e > theta * norm / std::sqrt(n); };

CHECK(std::ranges::count_if(marker, mark)
== static_cast<std::int32_t>(indices.size()));

for (std::int32_t i = 0; i < static_cast<std::int32_t>(marker.size()); ++i)
{
bool expect_marked = mark(marker[i]);
bool marked = std::ranges::find(indices, i) != indices.end();
CHECK(expect_marked == marked);
}
}
2 changes: 2 additions & 0 deletions python/dolfinx/mesh.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
from dolfinx.cpp.refinement import (
IdentityPartitionerPlaceholder,
RefinementOption,
mark_equidistribution,
Comment thread
jhale marked this conversation as resolved.
Outdated
mark_maximum,
)
from dolfinx.cpp.refinement import (
Expand Down Expand Up @@ -73,6 +74,7 @@
"exterior_facet_indices",
"locate_entities",
"locate_entities_boundary",
"mark_equidistribution",
"mark_maximum",
"meshtags",
"meshtags_from_entities",
Expand Down
11 changes: 11 additions & 0 deletions python/dolfinx/wrappers/dolfinx_wrappers/refinement.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,17 @@ void declare_refinement(nanobind::module_& m)
std::span(marker.data(), marker.size()), theta, comm.get()));
},
nb::arg("marker"), nb::arg("theta"), nb::arg("comm"));

m.def(
"mark_equidistribution",
[](nb::ndarray<const T, nb::ndim<1>, nb::c_contig> marker, T theta,
MPICommWrapper comm)
{
return dolfinx_wrappers::as_nbarray(
dolfinx::refinement::mark_equidistribution(
std::span(marker.data(), marker.size()), theta, comm.get()));
},
nb::arg("marker"), nb::arg("theta"), nb::arg("comm"));
}

} // namespace dolfinx_wrappers
23 changes: 23 additions & 0 deletions python/test/unit/refinement/test_mark.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,26 @@
msh.topology.create_entities(1)
marked_edges = mesh.compute_incident_entities(msh.topology, marked_cells, tdim, 1)
mesh.refine(msh, marked_edges)


@pytest.mark.parametrize("theta", [0.2, 0.4, 0.6, 0.8])
@pytest.mark.parametrize("dtype", [np.float32, np.float64])
def test_mark_equidistribution(theta: float, dtype: np.dtype) -> None:
msh = mesh.create_unit_square(comm := MPI.COMM_WORLD, n := 10, n, dtype=dtype)

Check warning on line 39 in python/test/unit/refinement/test_mark.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Move this assignment out of the argument list; ":=" operator is confusing in this context.

See more on https://sonarcloud.io/project/issues?id=FEniCS_dolfinx&issues=AZ3bN_tNqqJtg0faRuQD&open=AZ3bN_tNqqJtg0faRuQD&pullRequest=4187
Comment thread
schnellerhase marked this conversation as resolved.
Outdated

tdim = msh.topology.dim
cell_count = (cell_im := msh.topology.index_map(tdim)).size_local + cell_im.num_ghosts
marker = np.random.default_rng(0).random(cell_count)

marked_cells = mesh.mark_equidistribution(marker, theta, comm)

norm = np.sqrt(comm.allreduce(np.sum(marker**2), MPI.SUM))
count = comm.allreduce(marker.size)
assert np.allclose(
marked_cells,
np.argwhere(marker > theta * norm / np.sqrt(count)).flatten(),
Comment thread
schnellerhase marked this conversation as resolved.
Outdated
)

msh.topology.create_entities(1)
marked_edges = mesh.compute_incident_entities(msh.topology, marked_cells, tdim, 1)
mesh.refine(msh, marked_edges)
Loading