add mesh bp remove util by cyrush · Pull Request #1607 · llnl/conduit

cyrush · 2026-05-15T19:18:56Z

add conduit::mesh::blueprint::remove, which allows you to remove components of a mesh (like topologies, fields, etc) along with there dependencies.

For example, removing a topology will remove any fields defined on that topology, removing a matset will remove references and matset specific values for any related fields.

cyrush · 2026-05-15T19:19:18Z

TODO: Test cases are WIP, need to add many more:

add multi domain test cases
add matset, adjset, and nestset test cases
make sure matset test cases include a field that with matset_values
think about other bad options scenarios

cyrush · 2026-05-18T21:40:18Z

presence of * as category name indicates that we should remove everything from that category

fields: ["den","radial"]
matsets: *
state: *

will fully remove state and all matsets, along with the fields den and radial

for state, other than *, any entry is assumed to be a path

fields: ["den","radial"]
matsets: *
state: time

cyrush · 2026-05-20T17:54:02Z

added specset test and python interface and python test

JustinPrivitera · 2026-05-20T18:03:44Z

There are many places we could leverage this (perhaps in Silo tests) that would provide additional testing coverage.

JustinPrivitera

Reviewing now. Are you going to add a changelog entry?

cyrush · 2026-05-20T21:05:34Z

Thanks, yes I will add Change log entry

…nduit into task/2026_05_mesh_bp_remove_util

JustinPrivitera

Love the lambdas. I have some comments about a few things.

JustinPrivitera · 2026-05-20T22:21:45Z

+                    std::regex comp_regex(comp_pat_adj);
+                    if(std::regex_match(comp_name, comp_regex))
+                    {
+                        //std::cout << "true" << std::endl;
+                        n_comp.remove(comp_name);
+                    }


I don't understand why we need to do regex match again. We already used it earlier to populate the set, so can't we just remove everything in the set unconditionally?

The set contains the regex, not the expanded names.

The prior walk explicitly records dependents, but the outer matches are not explicitly recorded.

I see. The set gets populated even further above in init_set_from_opts.

JustinPrivitera · 2026-05-20T22:24:58Z

+    // past to clean up matsets
+    // matsets: for any field that refs a matset,
+    //          remove that ref and any matset values
+
+    for(size_t i = 0; i < domains.size(); i++)
+    {
+        conduit::Node *dom = domains[i];


Why not remove fields that depend on a matset entirely? The field may or may not be in a valid state after the operation you've implemented here. If the field is material-dependent, then it will still be valid, but if it is mixed (https://llnl-conduit.readthedocs.io/en/latest/blueprint_mesh.html#fields), then it is no longer valid and should be removed entirely.

fully material-dependent is rare, usually values exists and is useful even without the matset values. That said, I this should check for that case.

Matset values can be very large, removing them when not needed is a savings if folks only want to save or process the overall field values.

check for material-dependent case (no values), and remove field if when no useful data is left over.

I think it is sufficient to check for the existence of values after scrubbing the matset association, and remove if there are no values. However, fields are handled differently from everything else in remove. Maybe there is an option to aggressively remove fields versus conservatively removing them? It just seems like an unexpected behavior of the function. If I am removing a matset would I not want associated fields to also be removed? I think both choices are valid.

The relationship between fields and matsets is a bit different.

I think removing the entire field is too aggressive in this case b/c it removes data that is still useful.
Most viz uses cases will use values and never touch matset values unless the user is doing something sophisticated.

They can always remove any field fully by name or pattern.

JustinPrivitera · 2026-05-20T22:28:08Z

+        // removing the matset should remove the specset as well
+        EXPECT_TRUE(n_mesh.has_path("matsets/mesh"));
+        EXPECT_FALSE(n_mesh.has_path("specsets/mesh"));


Suggested change

// removing the matset should remove the specset as well

EXPECT_TRUE(n_mesh.has_path("matsets/mesh"));

EXPECT_FALSE(n_mesh.has_path("specsets/mesh"));

// removing the matset should remove the specset as well

EXPECT_FALSE(n_mesh.has_path("matsets/mesh"));

EXPECT_FALSE(n_mesh.has_path("specsets/mesh"));

looks like this was copied from above and not updated to reflect the comment. It doesn't test what the comment says it does.

It is a copy and paste problem -- but the above comment is what is wrong.
In this case we are removing the specset, that matset should still exist.

No, look closer - this test is copied from the case right before it. Right now both tests are identical. It makes sense to test three cases for specsets:

removing a specset, which is tested twice

removing a matset which should remove the specset also, which your comment suggested you wanted to test, but is not tested as things currently stand

removing a topo which should remove the matset which should remove the specset, which is tested at the end

I think you want to keep the original comment and fix the options that are tested and the resulting EXPECT_***.

thank, you I think have this sorted out now. Can you take one more look?

yes looks good

JustinPrivitera · 2026-05-20T22:30:43Z

currently we can either give a name or give "*"? Do we want to support "*_name" or any other regex stuff?

See my comment about glob vs regex, I will work on support for that so that * isn't a special case

Co-authored-by: Justin Privitera <35237779+JustinPrivitera@users.noreply.github.com>

cyrush · 2026-05-21T15:48:24Z

For filtering, what we really want is glob matching, not regex matching.

We can convert glob style matching into a regex, I will add a utility to do that.

This will also be useful in other cases (extending mesh blueprint read to read subsets of a mesh with same style of args used here)

JustinPrivitera · 2026-05-21T16:31:29Z

+    {
+        conduit::blueprint::mesh::examples::generate("misc",n_mesh);
+        EXPECT_TRUE(n_mesh.has_path("topologies/mesh"));
+        EXPECT_TRUE(n_mesh.has_path("fields/radial"));
+        EXPECT_TRUE(n_mesh.has_path("matsets/mesh"));
+        EXPECT_TRUE(n_mesh.has_path("specsets/mesh"));
+
+        n_opts.reset();
+        n_opts["specsets"] = "mesh";
+
+        echo_node("[before]",n_mesh);
+        echo_node("[options]",n_opts);
+        conduit::blueprint::mesh::remove(n_opts,n_mesh);
+        EXPECT_TRUE(conduit::blueprint::mesh::verify(n_mesh, n_info));
+        echo_node("[after]",n_mesh);
+        echo_node("[info]",n_info);
+
+        // after removing specset, matset should remain
+        EXPECT_TRUE(n_mesh.has_path("matsets/mesh"));
+        EXPECT_FALSE(n_mesh.has_path("specsets/mesh"));


this test should be updated to test removing a matset: #1607 (comment)

I think this is sorted, please give it one more look (sorry for the double comment)

yes looks good

cyrush · 2026-05-21T18:23:50Z

I created a glob match util and remove refactored to use it.

wip: add mesh bp remove util

7622cb8

more logic and more tests

eed5924

cyrush added 6 commits May 18, 2026 16:47

state support, wild card support

41c2de8

use regex to expand cap to match and simplify overall logic

11a1040

expand tests

55667b2

try catch translate for py mesh bp example gen

3cd8fd1

python iface updates

9b6655b

updates

e51275b

cyrush requested a review from JustinPrivitera May 20, 2026 19:01

cyrush commented May 20, 2026

View reviewed changes

Comment thread src/tests/blueprint/t_blueprint_mesh_remove.cpp

Apply suggestion from @cyrush

1a14697

JustinPrivitera reviewed May 20, 2026

View reviewed changes

cyrush added 5 commits May 20, 2026 14:11

fix bug

5f6df29

Merge branch 'task/2026_05_mesh_bp_remove_util' of github.com:LLNL/co…

fa467ef

…nduit into task/2026_05_mesh_bp_remove_util

changelog

18f6560

fix and cleanup

06fbfcb

Merge branch 'develop' into task/2026_05_mesh_bp_remove_util

341bcd1

JustinPrivitera requested changes May 20, 2026

View reviewed changes

Update src/libs/blueprint/conduit_blueprint_mesh.cpp

d7cb353

Co-authored-by: Justin Privitera <35237779+JustinPrivitera@users.noreply.github.com>

cyrush commented May 21, 2026

View reviewed changes

Comment thread src/tests/blueprint/t_blueprint_mesh_remove.cpp

Update src/tests/blueprint/t_blueprint_mesh_remove.cpp

13b8c3a

JustinPrivitera reviewed May 21, 2026

View reviewed changes

cyrush added 2 commits May 21, 2026 10:38

fix specset matset dep case

4b063ca

centralize glob logic

463f390

cyrush added 2 commits May 21, 2026 12:35

add matset mat dep field support and test, add test titles

7e5cfb1

msvc c++17 issue

2467d68

JustinPrivitera approved these changes May 21, 2026

View reviewed changes

finish new loop,fix typo in arg

42f8438

cyrush merged commit 1890886 into develop May 22, 2026
25 of 26 checks passed

cyrush deleted the task/2026_05_mesh_bp_remove_util branch May 22, 2026 16:38

cyrush mentioned this pull request Jun 3, 2026

rename and remove helpers utils for coordset, topology, etc #940

Closed

Conversation

cyrush commented May 15, 2026

Uh oh!

cyrush commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cyrush commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cyrush commented May 20, 2026

Uh oh!

JustinPrivitera commented May 20, 2026

Uh oh!

Uh oh!

JustinPrivitera left a comment

Choose a reason for hiding this comment

Uh oh!

cyrush commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JustinPrivitera left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JustinPrivitera May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cyrush commented May 21, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cyrush commented May 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cyrush commented May 15, 2026 •

edited

Loading

cyrush commented May 18, 2026 •

edited

Loading

cyrush commented May 20, 2026 •

edited

Loading

JustinPrivitera May 21, 2026 •

edited

Loading