Generate 1D strip matsets #1038
Conversation
| @@ -642,6 +650,11 @@ namespace topology | |||
| //----------------------------------------------------------------------------- | |||
| namespace matset | |||
| { | |||
| //------------------------------------------------------------------------- | |||
| void CONDUIT_BLUEPRINT_API generate_strip(conduit::Node& fields, | |||
There was a problem hiding this comment.
This should be a private helper b/c it really can't be used outside the context of other generate_strip methods.
There was a problem hiding this comment.
Further review: don't think we need this this guy?
(Not being called)
Sorry if I am commenting while things are evolving.
|
Need to add tests |
|
Putting up this PR because it turns out we need matsets right away. |
| } | ||
|
|
||
| // check for target matset names | ||
| if (options.has_child("matset_names")) |
There was a problem hiding this comment.
Note: matset names could be inferred from the field_names options (or all fields).
I expect thats how folks would think to use it ("I want these fields, make it so")
|
|
||
| // generate new coordset, topology, and fields | ||
| mesh::generate_strip(topo, topo_dest, coords_dest, fields_dest, options); | ||
|
|
There was a problem hiding this comment.
For any new field that has a matset, we need to rewire:
fields/{name}/matset = newmatsetname
|
Matsets refer to a topology. All fields refer to a topology, and some fields refer to a matset. Here's my question, @cyrush : Will we ever have a matset that does not get referred to by a field? Do we care about those matsets? It's much easier if we can ignore "orphan" matsets that refer to the source topo but aren't referred to by any field. In that case, as you suggested on line 2287 of conduit_blueprint_mesh.cpp, we can just use options/field_prefix to name the new matsets. I guess we could use options/field_prefix to name all the new matsets that get created for the new topology. Thoughts? |
|
For the intended use case, I believe folks will care first and foremost about mapping fields ("I need these fields") To give full control, let's provide a |
|
Another question: A common case (that we are showing in the example) is for the user to create the new topo and all dependent fields, etc. in the same mesh. In this case, unless the user specifies a fields_prefix, there will be overwritten fields. Do we care? Should we make a default fields_prefix? I mean, perhaps the user meant to overwrite the field, though I really doubt that. |
|
I think overwriting fields (and other components) will be unintended in most all cases, and could be a source of pain. We should error if folks try to override any field, or matset. We could also guard the topology or coordset, but those checks will require different implementation. |
|
We need to exercise this on a test. This opens up a huge can of worms. Only one of the meshes produced by the functions in conduit_blueprint_examples.cpp has a matset, and that one isn't linked to a field. We could add matsets to basic(); I think that would definitely be scope creep. We could add a top level (public) function to conduit_blueprint_mesh_examples.hpp/cpp, named something like And then there is the fact that we have several different forms of matset. I would propose to generate the form that the user needs, then later add the others. |
|
We do need test data to exercise this -- but we don't need a fully fledged example. Since we are doing a simple 1D dataset as input, I recommend we create a matset and field. We effectively copy the matset so I am not worried about testing all the combos of the matset internals. |
Agreed!
OK, so I'll build the matset in the test.
Yes, I'll build the kind of matset that we're interested in. |
|
Good plan. |
Similar to fields, new matsets must be generated that point to the new topologies.