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

write a module describing orientation / general orientatation cleanup #14667

Open
7 of 12 tasks
drwells opened this issue Jan 11, 2023 · 2 comments
Open
7 of 12 tasks

write a module describing orientation / general orientatation cleanup #14667

drwells opened this issue Jan 11, 2023 · 2 comments
Assignees
Milestone

Comments

@drwells
Copy link
Member

drwells commented Jan 11, 2023

We should put this information all in one place. A few relevant classes/functions/etc:

  1. TriaObjectsOrientations
  2. ReferenceCell
  3. GeometryInfo
  4. the simplex module
  5. make_periodicity_constraints
  6. QProjector (a lot of orientation information is hard-coded into this class)
  7. GridTools::orthogonal_equality (this one uses std::bitset<3>)
  8. PeriodicFacePair (also uses bitset)

In a similar vein, we should settle on 'combined orientation' or 'raw orientation' - I recall seeing both in the library

Consistency problems:

  • combined vs raw orientation (we should settle on a single name)
  • different conventions for converting (bool, bool, bool) to a single integer flag (e.g., in FiniteElement)
  • MatrixFree wants the standard orientation to be all zeros, whereas 1u is the default orientation in most of the rest of the library
  • we should not use both unsigned char and std::bitset<3>
  • Standardize on using face_orientation as the sole relevant bit in 2D. Many parts of the library (e.g., GeometryInfo) use face_flip instead.
  • Standardize on using the order 'orientation, rotation, flip'. The std::bitset<3> version uses 'orientation, flip, rotation'.
  • Add utility functions for getting reverse orientations instead of hard-coding it
  • We sometimes assume that things are always in the standard orientation in 2d. We should remove this assumption because it doesn't work for mixed meshes (i.e., FE_Q cannot make this assumption since it does not know whether or not it is part of a mixed mesh).
  • Introduce types::orientation_type or something similar instead of using unsigned char
  • Clean up the documentation of QProjector. It was written long ago and the style is difficult to understand (it focuses too much on how and not enough on why)
  • FE_ABF doesn't actually loop over the different possible face orientations - it used (false, false, false) (which is almost surely wrong) to initialize some values. Take a closer look at how it computes things.
  • Take another look at face_to_cell_index(): maybe we have enough infrastructure now to generalize everything.
@bangerth
Copy link
Member

It should be in ReferenceCell. That's where we've been moving all of the other stuff that relates to geometric descriptions of reference cells.

@drwells
Copy link
Member Author

drwells commented Apr 5, 2023

See also #15025.

This was referenced Apr 20, 2024
drwells added a commit to drwells/dealii that referenced this issue Apr 20, 2024
Partially reverts dealii#15678.

While dealii#15678 fixed the permutation bug, it didn't address the true cause of the
problem: since face orientations are computed in the "apply this permutation to
face 1 to get face 2" direction, QProjector should use inverse orientations.
Since 2d orientations are their own inverses this only shows up in 3d.

Whenever two faces abutt, the first face is always in the default orientation
and the second face's orientation is computed relative to that (as decribed
here). Hence, when we project quadrature points onto the first face they do not
need to reoriented. However, we need to apply the *reverse* permutation on the
second face so that they end up in the same positions as the first face. More
formally: most, but not all, orientations are their own inverses. In particular,
triangle orientations 3 and 5 are each-other's inverses.

This matches the notion of inverse orientation used in dealii#16828 for hypercubes. In
the future, we should combine the hypercube and non-hypercube implementations to
avoid these kinds of inconsistencies.

Part of dealii#14667.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants