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

Symmetrize #99

Merged
merged 18 commits into from
Jun 14, 2024
Merged

Symmetrize #99

merged 18 commits into from
Jun 14, 2024

Conversation

DavidAKopriva
Copy link
Collaborator

Add capability to take a mesh and reflect it about a (straight line) boundary with the name "symmetry". This should answer the request of some users for a symmetric mesh. TODO: Add example, testing, documentation, and test the mesh files.

Add routine to see if a curve is a straight line by checking the second derivative at a point. Add tests for that, too. See comments for limitations.
Start adding code to mirror across a symmetry line.
Add code to compute the reflection of a point about a line, plus tests. Put in stubs for mirroring a mesh.
Add changes to flip a mesh about a symmetry line and add the new elements to the mesh. Only does one symmetry boundary in the model ATM. Haven't checked the mesh file yet, and edges need to be added still to handle ISM-V2.
Add reflection of the edges in the mesh by removing the old ones and re-computing them. Also reflect the xPatch array, which is used in the SEM plotting procedure. Finally update the makefile to include the new reflection code.
Copy link

codecov bot commented Jun 6, 2024

Codecov Report

Attention: Patch coverage is 95.05300% with 14 lines in your changes missing coverage. Please review.

Project coverage is 74.88%. Comparing base (80ac66f) to head (6e3e480).

Files Patch % Lines
Source/Project/Model/SMModel.f90 75.60% 10 Missing ⚠️
Source/Mesh/MeshGeneratorMethods.f90 97.69% 3 Missing ⚠️
Source/HOHQMesh.f90 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #99      +/-   ##
==========================================
+ Coverage   74.19%   74.88%   +0.69%     
==========================================
  Files          67       68       +1     
  Lines       10155    10429     +274     
  Branches        2        2              
==========================================
+ Hits         7534     7810     +276     
+ Misses       2621     2619       -2     
Flag Coverage Δ
unittests 74.88% <95.05%> (+0.69%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

DavidAKopriva and others added 10 commits June 6, 2024 17:00
Fix ifort error regarding class/type of a variable. Fix typos.
Add routines to test if all curves marked symmetry are co-linear straight lines. If not, posts a warning and the mesh is not reflected.
Fixes a longstanding error in assigning the boundary curve to an element boundary edge. Instead of using ROW_END as a discriminant to choose which curve the two nodes fall on (a corner node can be on two curves simultaneously), it checks to see if the node is at the endpoints of its curve and if so, uses the curve attached to the other node.
Make boundary curves int the boundaryInfo consistent with the defined directions.
Added PacMan example doing symmetry reflection to test suite. Change test on SYMMETRY_CURVE_NAME to be case insensitive.
Update the documentation to include the PacMan example.
Update to the previous to include one more file.
Append "_R" to boundary names that are on the reflected portion of the model domain, to allow for different BCs if desired.
@DavidAKopriva
Copy link
Collaborator Author

I've updated everything I can think of. One thing especially open for discussion is that I append "R" to a boundary name on the reflected portion of the mesh. I guess if one does not want that, it's a quick search/replace (maybe) in the file. Maybe something else truly unique would be better ? "#R?

Copy link
Member

@andrewwinters5000 andrewwinters5000 left a comment

Choose a reason for hiding this comment

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

Looks good overall; just a small suggestion with the literals.

One follow-up question. If one sets multiple symmetry boundary it only reflects the "base" mesh, right? It would not be the case that the base mesh gets reflected over one boundary and the entire mesh is then reflected over the second boundary?

Source/Foundation/LineReflectionModule.f90 Outdated Show resolved Hide resolved
Add literal type where it was missing.

Co-authored-by: Andrew Winters <[email protected]>
Copy link
Member

@andrewwinters5000 andrewwinters5000 left a comment

Choose a reason for hiding this comment

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

LGTM! Once this is merged and released I need to add the capability to the HOHQMesh.jl counterpart and add some test and a tutorial. (This is mostly a written note to myself more than anything, you do not need to do anything.)

@DavidAKopriva
Copy link
Collaborator Author

DavidAKopriva commented Jun 13, 2024

No, there's nothing that you have to do. Just call at least one of the boundary curves "symmetry" when you are generating a model and off it will go. (Hey, this might be a way to define an interior boundary! If one of the curves along a line is called symmetry and the others aren't, then the nodes will be duplicated, along those others and the curve names are not deleted. Hmm.) This is what one user was asking for at one point. This does work, but messages saying that there are duplicate nodes are posted. That's OK, because one expects that to happen. Huh.

@andrewwinters5000
Copy link
Member

Reflection

@andrewwinters5000 andrewwinters5000 merged commit f7a5e6c into main Jun 14, 2024
17 checks passed
@andrewwinters5000 andrewwinters5000 deleted the Symmetrize branch June 14, 2024 18:26
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