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

Orientation rotation flip #10326

Merged
merged 2 commits into from May 24, 2020
Merged

Conversation

peterrum
Copy link
Member

@peterrum peterrum commented May 23, 2020

Merge orientation/flip/rotation to a single char with the meaning of the bits:

  • bit 0: orientation
  • bit 1: flip
  • bit 2: rotation

In the case of other mesh entity types these number might have a different meaning.

Note: This is solely an internal change. The TriaAccssors still return the same information.

Copy link
Member

@tamiko tamiko left a comment

Choose a reason for hiding this comment

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

This looks reasonable.
We have quite a number of tests for face orientation/rotation/flip. Let's run the full testsuite and see what happens.

@tamiko
Copy link
Member

tamiko commented May 23, 2020

/rebuild

@tamiko tamiko added this to the Hackathon 2020 milestone May 23, 2020
Copy link
Member

@kronbichler kronbichler left a comment

Choose a reason for hiding this comment

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

Nice simplification. One thing I don't have clear for myself is why there is this asymmetry in the access to the orientation of a face versus the flip and rotation; the former gets queried through the TriaObjects{Quad,Hex}3D whereas the latter two are only in the accessor.

@peterrum peterrum added this to In progress in Triangulation generalizations via automation May 23, 2020
@drwells drwells merged commit 22ccf70 into dealii:master May 24, 2020
Triangulation generalizations automation moved this from In progress to Done May 24, 2020
Copy link
Member

@bangerth bangerth left a comment

Choose a reason for hiding this comment

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

Out of curiosity: you're doing this because you want to generalize it for triangles/tets?

It doesn't matter much in the grand scheme of things, but a vector<bool> really just stores one bit per element. So having three such vectors requires less memory than storing one vector<char>.

@bangerth
Copy link
Member

On the plus side, the vector<bool> specialization is deprecated and we should avoid using it anyway :-)

@peterrum
Copy link
Member Author

Out of curiosity: you're doing this because you want to generalize it for triangles/tets?

Exactly. This is a preparation for that.

It doesn't matter much in the grand scheme of things, but a vector really just stores one bit per element. So having three such vectors requires less memory than storing one vector.

I was not aware that vector<bool> has its own specialization. Now, it makes sense why I had issues with vector<bool> in the past!

Normally bool and char are equivalent in terms of size (just that bool only wastes 7 bits).

@bangerth
Copy link
Member

Yes, std::vector<bool> is a special case: https://en.cppreference.com/w/cpp/container/vector_bool

But it's not true that bool and char are equivalent in size. That actually depends on the platform, but on many platforms it is stored as a 32-bit data type for alignment reasons. (Not on x86_64, however.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

5 participants