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

HDF XDMF entry: store reference cell #14056

Merged
merged 5 commits into from Jul 6, 2022
Merged

Conversation

tjhei
Copy link
Member

@tjhei tjhei commented Jun 26, 2022

We added support for XDMF output of simplices in #10852 but ended up not storing the type of cell inside the XDMF entry but rely on passing it in after the fact. I think this is a mistake, so I am adding it here.

This is required for my upcoming fix for #13404

Copy link
Member

@drwells drwells left a comment

Choose a reason for hiding this comment

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

Some more things should be marked as early-deprecated.

Can this be easily extended to mixed meshes in the future?

* Deprecated constructor.
*
* @deprecated Use the constructor that additionally takes a ReferenceCell.
*/
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
*/
*/
DEAL_II_DEPRECATED_EARLY

or the other one.

default:
AssertThrow(false, ExcMessage("Invalid dimension"));
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Could we move this logic into a helper function and call it directly in the other ctor?

@tjhei
Copy link
Member Author

tjhei commented Jun 29, 2022

Some more things should be marked as early-deprecated.

I can do that, but these are mostly internal functions I would think so I thought deprecating directly is ok.

Can this be easily extended to mixed meshes in the future?

I only thought about this a bit, but it requires quite some work:

  1. xdmf supports "mixed" topology, but the cell "indices" will be quite different (you specify the cell type for every cell): https://xdmf.org/index.php/XDMF_Model_and_Format#Arbitrary
  2. As a consequence we will have a different "cells" table HDF5 layout (it will be 1d instead of 2d)
  3. HDF5 output builds on DataOutFilter, which doesn't do mixed meshes (or I don't understand the internals)

Regarding this PR: I would think we would have a type of xdmf entry for mixed meshes (maybe using cell_type=invalid) which writes the correct information.

@drwells
Copy link
Member

drwells commented Jun 29, 2022

If its internal we can deprecate it normally.

Thanks for checking the mixed mesh situation - it sounds like we should deal with it later.

@tjhei
Copy link
Member Author

tjhei commented Jun 29, 2022

Thanks for checking the mixed mesh situation - it sounds like we should deal with it later.

It is certainly non-trivial, but I can not promise that we need to change things this PR touches.

@masterleinad masterleinad merged commit 197286a into dealii:master Jul 6, 2022
@tjhei tjhei deleted the xdmf_ref_cell branch July 6, 2022 17:21
mkghadban pushed a commit to OpenFCST/dealii that referenced this pull request Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants