LLNL / conduit

Simplified Data Exchange for HPC Simulations
https://software.llnl.gov/conduit/
Other
212 stars 65 forks source link

Polyhedral/Polygonal support for stream-based mesh blueprint - how to best do this? #971

Closed mennodeij closed 2 years ago

mennodeij commented 2 years ago

Good morning,

I'm working on adding support for mixed cell streams to the VTK/ParaView implementation of Conduit/MeshBlueprint for in-situ processing. A merge request is pending at the moment , but before this is merged I wanted to check here if my approach is valid when using element_counts or offsets. Two examples are given below for these two approaches.

My question is, other polyhedral cell examples have sub_elements for the faces. Below I have not done that, and the whole cell is defined in the stream with the VTK approach, i.e. nfaces, npoints_face1, pt_0, pt_1, ... ptN, npoints_face2, pt_0, pt_1, ..., ptM, ...

Is this a good approach, or would you prefer the approach with sub_elements?

Polyhedral example for three polyhedra with element_counts:

coordsets:
  coords:
    type: "explicit"
    values:
      x: [0.0, 1.0, 1.0, 0.0, 0.0, 1.0, 1.0, 0.0, 0.5, 0.5]
      y: [0.0, 0.0, 1.0, 1.0, 0.0, 0.0, 1.0, 0.0, 0.5, 0.5]
      z: [0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, -1.0, 2.0]
topologies:
  mesh:
    type: "unstructured"
    coordset: "coords"
    elements:
      element_types:
        polyhedral:
          stream_id: 0
          shape: "polyhedral"
      element_index:
        stream_ids: 0
        element_counts: 3
      stream: [5, 4, 4, 5, 6, 7, 3, 4, 5, 9, 3, 5, 6, 9, 3, 6, 7, 9, 3, 7, 4, 9, 5, 4, 0, 1, 2, 3, 3, 0, 1, 8, 3, 1, 2, 8, 3, 2, 3, 8, 3, 3, 0, 8, 6, 4, 0, 1, 4, 4, 4, 1, 2, 6, 5, 4, 2, 3, 7, 6, 4, 3, 0, 4, 7, 4, 0, 1, 2, 3, 4, 4, 5, 6, 7]

2) Same example but with offsets:

coordsets:
  coords:
    type: "explicit"
    values:
      x: [0.0, 1.0, 1.0, 0.0, 0.0, 1.0, 1.0, 0.0, 0.5, 0.5]
      y: [0.0, 0.0, 1.0, 1.0, 0.0, 0.0, 1.0, 0.0, 0.5, 0.5]
      z: [0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, -1.0, 2.0]
topologies:
  mesh:
    type: "unstructured"
    coordset: "coords"
    elements:
      element_types:
        polyhedral:
          stream_id: 42
          shape: "polyhedral"
      element_index:
        stream_ids: [42, 42, 42]
        offsets: [0, 22, 44, 75]
      stream: [5, 4, 4, 5, 6, 7, 3, 4, 5, 9, 3, 5, 6, 9, 3, 6, 7, 9, 3, 7, 4, 9, 5, 4, 0, 1, 2, 3, 3, 0, 1, 8, 3, 1, 2, 8, 3, 2, 3, 8, 3, 3, 0, 8, 6, 4, 0, 1, 4, 4, 4, 1, 2, 6, 5, 4, 2, 3, 7, 6, 4, 3, 0, 4, 7, 4, 0, 1, 2, 3, 4, 4, 5, 6, 7]

These examples validate in blueprint.

cyrush commented 2 years ago

@mennodeij The steam_id + stream concept is an older one that we never fully adopted. It was heavily influenced by VTK's in memory connectivity format, but folks found it confusing in practice.

Right now, only official way to do mixed element types is a full polyhedral description using sizes, offsets, and subelements.

The polytess and polychain example outputs provide good context:

https://llnl-conduit.readthedocs.io/en/latest/blueprint_mesh.html#polytess

https://llnl-conduit.readthedocs.io/en/latest /blueprint_mesh.html#polychain

Since full polyhedral is a complex way to defined mixed element meshes, we do want a simpler path for known zoo style elements.

But it sounds like you may be aiming for Polyhedral regardless? If that is the case, the Polyhedral examples above should show you exactly what you need.

We haven't prioritized non-polyhedral mixed element topologies yet, but we do have the o2m construct which (used in several places now) that we want to leverage. https://github.com/LLNL/conduit/issues/710.

Hopefully this helps! Let me know if you have more questions.

mennodeij commented 2 years ago

@cyrush Thank you for your detailed answer.

My initial impression was the opposite, that the stream_id concept was something new that wasn't yet documented. Good to check here about it. Is it something that you will keep, or will it be removed at some point?

Currently we have an implementation for in-situ processing in our flow code with conduit/meshblueprint and paraview that uses the full polyhedral approach based on the examples given, but I'd like to go to a mixed cell approach (zoo style elements is not a term I'm familiar with, but do you mean 'simple' elements, i.e. hex, tet, etc?). The mixed cell definition can contain some polyhedra. I prefer the mixed cell approach as downstream processing (clipping, contouring, etc.) is much faster and robust on simple elements than on polyhedra.

How would the o2m approach be used in a mixed cell approach? Can I help to move that forward?

cyrush commented 2 years ago

Sorry, yes by zoo style finite elements I mean common elements like triangles, quads, tets, hexs -- vs arbitrary polygonal/polyhedral.

With the o2m approach, I think that main change would be that the stream_ids would not be embedded in the connectivity.

The current stream_id looks like:

[stream_id_0 , element_0_idx_first, ..., element_0_idx_last, stream_id_1, element_1_idx_first, ..., element_1_idx_last]

We would instead have the stream_ids in a separate array and use offsets strides, (and indices if needed) arrays to describe the connectivity values.

This would then leverage the one-2-many (O2M) relationship blueprint spec (https://llnl-conduit.readthedocs.io/en/latest/blueprint_o2mrelation.html)

We will need to think about final names for the steam_ids, etc -- but I think this is the basic path we should take.

Let me know if this makes sense & lets keep discussing!

mennodeij commented 2 years ago

Ok, clear. My first proposal to support this would be this:

topologies:
  mesh: "unstructured"
    coordset: "coords"
    elements:
      element_types:
        mixed:
          shape: "mixed"
      element_index:
        cell_types: [ VTK_HEXAHEDRON, VTK_TETRA, VTK_POLYHEDRON ]
        sizes: [ 8, 4, 7 ]
        offsets: [ 0, 8, 12, 19 ]
        indices: [ 0, 1, 3, 2, 4, 5, 7, 6,  0, 1, 2, 3,  0, 1, 2, 3, 4, 5, 6 ] 
    sub_elements:
      element_types:
        mixed:
          shape: "mixed"
      element_index:
        cell_types: [ VTK_TRIANGLE, VTK_QUAD, VTK_TRIANGLE, VTK_TRIANGLE, VTK_QUAD, VTK_POLYGON, VTK_TRIANGLE ]
        sizes: [ 3, 4, 3, 3, 4, 6, 3 ]
        offsets: [ 0, 3, 7, 10, 13, 17, 23, 26 ]
        indices: [ 0, 1, 2,  2, 3, 4, 0,  2, 3, 5,  3, 5, 6,  4, 5, 7, 6,  4, 6, 7, 8, 2, 3,  7, 8, 9 ] 

(Note 1: actual indices are made up and only serve illustration purpose). (Note 2: VTK_* cell types are written out, but in practice will of course be replaced by their numeric value)

So to summarize:

I'm looking forward to feedback on this. Meanwhile I'm perusing the conduit_blueprint codebase to see how to implement this.

cyrush commented 2 years ago

Yes, here is a suggested iteration, let me know what you think:

topologies: 
mesh: 
  type: "unstructured"
  coordset: "coords"
  elements: 
    # identify we are using multiple element types 
    shape: "mixed"
    # provide map from blueprint names to ids used in the "elements/shapes"
    shape_map: 
        # note, we would use actual (integer) values for VTK_ZZZ enum vals
        hex:  VTK_HEXAHEDRON 
        tet: VTK_TETRA 
        polyhedron: VTK_POLYHEDRON
    # array with shape ids for elements 0,1,2 ...
    shapes:  [ VTK_HEXAHEDRON, VTK_TETRA, VTK_POLYHEDRON ]
    # this is the o2m, sizes + offsets (that describe how to access of connectivity)
    sizes: [ 8, 4, 7 ]
    offsets: [ 0, 8, 12, 19 ]
    connectivity: [ 0, 1, 3, 2, 4, 5, 7, 6,  0, 1, 2, 3,  0, 1, 2, 3, 4, 5, 6 ]
  subelements:
    # identify we are using multiple element types 
    shape: "mixed"
    # provide map from blueprint names to ids used in the "subelements/shapes"
    shape_map: 
        # note, we would use actual (integer) values for VTK_ZZZ enum vals
        tri:  VTK_TRIANGLE
        quad: VTK_QUAD
        polygon: VTK_POLYGON
    # array with shape ids for subelements 0,1,2 ...
    shapes:  [ VTK_TRIANGLE, VTK_QUAD, VTK_TRIANGLE, VTK_TRIANGLE, VTK_QUAD, VTK_POLYGON, VTK_TRIANGLE ]
    # this is the o2m, sizes + offsets (that describe how to access of connectivity)
    sizes: [ 0, 3, 7, 10, 13, 17, 23, 26 ]
    offsets: [ 0, 3, 7, 10, 13, 17, 23, 26 ]
    connectivity: [ 0, 1, 2,  2, 3, 4, 0,  2, 3, 5,  3, 5, 6,  4, 5, 7, 6,  4, 6, 7, 8, 2, 3,  7, 8, 9 ] 

Changes:

mennodeij commented 2 years ago

Thank you for the suggestion and improvements, I like it.

I have one question though, I was playing a bit with the o2mrelation and its validation (see conduit::blueprint::o2mrelation::verify in conduit_blueprint_o2mrelation.cpp) yesterday, and I noticed that for a node to support the o2mrelation format, it must contain at least one 'data' array and the three arrays called sizes, offsets and indices. This is the reason for the choice in my first attempt to have the element_index node contain cell_types (the data) and the three o2mrelation arrays. I suppose that renaming connectivity in your proposed format would go a step in the right direction, but would that then validate as an o2mrelation with the shapes array as data, or would the shapes and o2mrelation arrays need to be grouped under a separate node, like it was in the element_index?

Another thing, ideally I'd like to support VTK_WEDGE and VTK_PYRAMID (i.e. prism and pyramidal cell types) as well, but I don't see these in the meshblueprint code base. Having them in the shapes array can be done of course, with the consumer of that array having the resposibility to support these additional cell types. The alternative is to have these cell types defined as polyhedra of course. What do you think?

cyrush commented 2 years ago

Yes, the hope is that the connectivity array serves as the data in the o2m

I can see that adding the shapes at the same level might tangle with the o2m.

I will try to look at a few possible cases in practice and see what is possible.

For wedge and pyramid, we do want to add these as officially supported element types:

https://github.com/LLNL/conduit/issues/132

It's easy to add them as new cases to blueprint verify -- but I also want an example (a new case of the braid example) that can serve as a concrete data do demo the new cases.

mennodeij commented 2 years ago

I propose to continue the discussion in #977 .