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

Updates to several overset update routines #330

Merged
merged 29 commits into from
Jun 28, 2024

Conversation

anilyil
Copy link
Contributor

@anilyil anilyil commented Nov 28, 2023

Purpose

This PR contains several changes to the overset cutting algorithm. These can be grouped as:

  1. Surface Callback Tracking: In Several features on tecplot io, slices, MG PC, cavitation constraints, and overset hole cutting. #231, I introduced a new way to do the overset hole cutting. This method introduced the use of blanking surfaces that can be used instead of the regular flooding algorithm for difficult cases. In the current PR, I cleaned up that code a bit and enabled "DVGeo tracking" for these surfaces. So as the main CFD mesh is deformed, the blanking surface is deformed consistently. This is useful when we need to re-run the full overset update after geometry changes. The DVGeo embedding is done in an embarrassingly parallel way because this blanking surface is duplicated on all procs.

  2. Changes to explicit surface cutting: Again, the feature mentioned above uses an algorithm under the hood to determine which cells are "inside" the closed surface. When these blanking surfaces are thin and they cut large cells relative to their thickness, some cells might slip from the cutting algorithm. To fix this, in this PR, I modified how the cells inside the surface are tagged. I first project the cell centers to the blanking surface. If the projection distance is greater than the max diagonal length of the cell, I end the test for that cell. If the projection distance does not satisfy this condition, we then check if the vector from the cell center to any of its vertices can potentially intersect the surface. If so, this cell is explicitly blanked.
    Compared to the old algorithm, this is a more conservative check; however, based on my experience, this approach produces good results and is not overly-conservative. The cells that are tagged should not be included in the solution anyways.

  3. Added 2 new options regarding overset updates:

  • updateWallAssociations: Enables updating the wall associations when geometry is updated, even when we are using the approximate wall distance routines. The overset meshes can only use the approximate wall distance routine, but before this PR, it was hard coded to always keep associations. In this PR, this behavior can be controlled by the user.
  • recomputeoverlapmatrix: In the full overset update mode, the block overlap matrix was not re-computed. If either the deformations are large, the CGNS block has many blocks, or both, this overlap matrix needs to be re-computed to get a valid hole cutting. In case someone still wants "efficient" full overset updates with small changes, they can use this option to disable the update. The default is set to True because it is somewhat likely for the overlap matrix to change when the mesh has large enough deformations that necessitates a full overset update.
  1. Updated how orphan cells are written to the CGNS volume file: Back in 2018, I added the capability to write a -5 for the cells that were considered as orphans in the hole cutting: fe9a015 However, I don't think this check was correct. There are actually two checks in that routine; the first check is for connectivity problems of compute cells. I think this check is triggered sometimes incorrectly for non-regular CGNS domain topologies (3, 5 point singularities etc). This check is where I was tagging the "orphans". However, I don't think this is the right definition.
    The following check is following the same logic in reverse; it starts from cells that are not compute or interpolate, and goes 2 cells in each direction. If a cell within this "radius" is compute, that cell is called an orphan cell.
    In this PR, I changed the orphan cell definition to be consistent with the second check. That check's result is what is printed as "number of orphans", and my old code would write more "orphan" cells in the volume solution than what is printed, causing confusion.

Unfortunately, I don't have tests for these routines. I am still planning to add some overset tests that go through all of these, but that will require some thinking and time. Until then, it would make my life a lot easier if we merge these changes.

Expected time until merged

1-2 months

Type of change

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (non-backwards-compatible fix or feature)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Documentation update
  • Maintenance update
  • Other (please describe)

Testing

Checklist

  • I have run flake8 and black to make sure the Python code adheres to PEP-8 and is consistently formatted
  • I have formatted the Fortran code with fprettify or C/C++ code with clang-format as applicable
  • I have run unit and regression tests which pass locally with my changes
  • I have added new tests that prove my fix is effective or that my feature works
  • I have added necessary documentation

@anilyil anilyil requested a review from a team as a code owner November 28, 2023 14:30
Copy link

codecov bot commented Nov 28, 2023

Codecov Report

Attention: Patch coverage is 25.67568% with 55 lines in your changes missing coverage. Please review.

Project coverage is 41.13%. Comparing base (b306045) to head (defb226).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
adflow/pyADflow.py 25.67% 55 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #330      +/-   ##
==========================================
- Coverage   41.42%   41.13%   -0.29%     
==========================================
  Files          13       13              
  Lines        4063     4108      +45     
==========================================
+ Hits         1683     1690       +7     
- Misses       2380     2418      +38     

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

@anilyil anilyil mentioned this pull request Nov 29, 2023
13 tasks
Copy link
Collaborator

@sseraj sseraj left a comment

Choose a reason for hiding this comment

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

Great changes! I tested the new explicit hole cutting algorithm on my mesh and can confirm that it is more conservative but also more effective. I just have a couple of comments

adflow/pyADflow.py Show resolved Hide resolved
src/overset/oversetAPI.F90 Outdated Show resolved Hide resolved
sseraj
sseraj previously approved these changes Jan 23, 2024
Copy link
Contributor

@eirikurj eirikurj left a comment

Choose a reason for hiding this comment

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

Sorry for the delay on this. Please seem my initial comments. At the moment, I dont have an overset mesh to test these changes right now. I think adding tests, even just some simple regression tests that touch this code, if possible, would be great.

adflow/pyADflow.py Outdated Show resolved Hide resolved
adflow/pyADflow.py Outdated Show resolved Hide resolved
adflow/pyADflow.py Outdated Show resolved Hide resolved
adflow/pyADflow.py Outdated Show resolved Hide resolved
doc/options.yaml Outdated Show resolved Hide resolved
src/inputParam/inputParamRoutines.F90 Show resolved Hide resolved
vertexPt = x(i - 1, j, k, :)
end select

if (cellID > 0) then
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this should be checked before the loop as nothing is done but run the loop otherwise. This should also not be necessary if IDs are checked upfront.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved the check to cover the blocks after cellID is computed.

src/overset/oversetAPI.F90 Show resolved Hide resolved
src/overset/oversetAPI.F90 Outdated Show resolved Hide resolved
src/overset/oversetUtilities.F90 Outdated Show resolved Hide resolved
@anilyil
Copy link
Contributor Author

anilyil commented Jun 20, 2024

@eirikurj I addressed your comments. I don't think I can add the overset tests now due to workload and this PR has been waiting for a long time already. I do want to add a series of overset tests but it would be great if we can do it separate from this PR.

@anilyil anilyil requested a review from eirikurj June 20, 2024 23:17
adflow/pyADflow.py Outdated Show resolved Hide resolved
@anilyil anilyil requested a review from eirikurj June 25, 2024 20:35
@eirikurj eirikurj requested review from a team, lamkina and marcomangano and removed request for a team June 26, 2024 10:21
@anilyil
Copy link
Contributor Author

anilyil commented Jun 26, 2024

@sseraj can you take a look at this again? I don't think there were many changes since last you checked.

Would it be possible for you to test if your grids initialize? No worries if not.

@sseraj
Copy link
Collaborator

sseraj commented Jun 27, 2024

Yes, I'll take another look at this and try to run my case as well

Copy link
Collaborator

@sseraj sseraj left a comment

Choose a reason for hiding this comment

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

The changes look good, and my case still runs the same as when I approved this PR previously

@sseraj
Copy link
Collaborator

sseraj commented Jun 28, 2024

Feel free to merge this (I would, but I took myself out of the maintainers group)

@eirikurj eirikurj merged commit 6706920 into mdolab:main Jun 28, 2024
16 of 17 checks passed
@anilyil anilyil deleted the surface_callback_tracking branch June 28, 2024 18:30
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.

4 participants