Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: read multiple Grids in XDMF #610

Closed
wants to merge 8 commits into from

Conversation

gdmcbain
Copy link
Contributor

@gdmcbain gdmcbain commented Jan 2, 2020

This is for #568.

@gdmcbain
Copy link
Contributor Author

gdmcbain commented Jan 2, 2020

This includes #609.

@gdmcbain
Copy link
Contributor Author

gdmcbain commented Jan 2, 2020

Thus far, this does read multiple Grids well enough to read the MWE; but perhaps this should go further and:

  • Distinguish the cells read from different grids.
  • Write multiple grids instead of TopologyType="Mixed".

The exact desired behaviour does depend on bit on the interpretation of tagged subsets of elements in meshio #290.

@gdmcbain
Copy link
Contributor Author

gdmcbain commented Jan 2, 2020

Given the current code

cells[xdmf_to_meshio_type[cell_type]] = data

plus the new code in #610 to read multiple Grids, subsequent grids with the same cell_type will clobber those preceding them. That can't be the desired behaviour. I suppose the new cells should be appended but should they be distinguished from those already read? How? Like Gmsh MSH2

cell_data[xdmf_to_meshio_type[cell_type]]["gmsh:physical"]

or something like MED for which I see

meshio/meshio/med/_med.py

Lines 105 to 114 in 23fa4ef

cell_tags = {}
if "ELEME" in fas:
cell_tags = _read_families(fas["ELEME"])
# Construct the mesh object
mesh = Mesh(
points, cells, point_data=point_data, cell_data=cell_data, field_data=field_data
)
mesh.point_tags = point_tags
mesh.cell_tags = cell_tags

though I'm not familiar with the MED format so don't know what that attribute would usually be expected to contain.

…Ah, I've just noticed that meshio.Mesh has sprouted some new attributes! node_sets and element_sets. I assume that these should be renamed point_sets and cell_sets (as per #493). The cell_sets could be just what's wanted here (and for #290).

@gdmcbain gdmcbain mentioned this pull request Jan 2, 2020
@gdmcbain
Copy link
Contributor Author

gdmcbain commented Jan 3, 2020

This now works again, following the switch from lxml, correctly reading the MWE of #568 with its two Grids sharing Geometry. It also records the two Topologies to cells and cell_sets (the latter partially fulfilling #290).

It remains to also write multiple grids, thereby:

  • avoiding TopologyType="Mixed"
  • enabling testing via test.helpers.write_read

@nschloe
Copy link
Owner

nschloe commented Jan 3, 2020

The smaller a PR, the better; we can do multiple grids later. Best bring this one here into a reviewable state.

@gdmcbain
Copy link
Contributor Author

gdmcbain commented Jan 5, 2020

Yes, right, sorry; I'll split this up.

@gdmcbain gdmcbain closed this Jan 5, 2020
This was referenced Jan 5, 2020
@gdmcbain
Copy link
Contributor Author

gdmcbain commented Jan 6, 2020

@gdmcbain
Copy link
Contributor Author

gdmcbain commented Jan 6, 2020

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.

2 participants