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

Overset tut #23

Merged
merged 39 commits into from
Mar 22, 2021
Merged

Overset tut #23

merged 39 commits into from
Mar 22, 2021

Conversation

DavidAnderegg
Copy link
Contributor

Purpose

This extends the MACH-Tutorial with a section for overset meshes. Additionally, geometry generation with OpenVSP and surface mesh generation with Pointwise is demonstrated.

Type of change

What types of change is it?

  • New feature (non-breaking change which adds functionality)

Testing

Checklist

Put an x in the boxes that apply.

  • 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

@DavidAnderegg
Copy link
Contributor Author

I believe, this is ready to get some feedback. There are still two things missing:

  • some reference-stuff doesnt work properly
  • I want to add one more picture at the start of the surface-mesh generation

I will fix those two things in the new year and would highly appreciate, if somebody could read it and tell me where i could improve.

@DavidAnderegg DavidAnderegg marked this pull request as ready for review December 21, 2020 11:48
@DavidAnderegg DavidAnderegg requested a review from a team as a code owner December 21, 2020 11:48
@ewu63 ewu63 force-pushed the master branch 2 times, most recently from eb6d339 to a6e13f5 Compare December 28, 2020 20:37
@DavidAnderegg
Copy link
Contributor Author

I fixed the the reference-stuff and decided, the picture i was planning to add, is not needed.

Copy link
Contributor

@marcomangano marcomangano left a comment

Choose a reason for hiding this comment

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

Hi David, thanks a lot for putting this big effort into this tutorial, it really looks good and it will definitely be useful.
I did not check the openVSP and Pointwise tutorials because I don't have the software, but I gave an overall look at the other sections and added a few comments for typos and minor additions (see below).
As additional comment, I would just recommend to not use the first person in the explanations, for consistency with the rest of the tutorials. Your contribution will still be evident via the about section in the main page, the links to adflow_utils, and the git log history!

Let me know if you want me and other students to check / try a section more in detail. I guess we need people to start using the tutorial extensively to catch all the fine details we can add or improve. This is already pretty close to be merged imho. Thanks again!

machAeroTutorials/overset_theory.rst Show resolved Hide resolved
machAeroTutorials/overset_theory.rst Show resolved Hide resolved
machAeroTutorials/overset_theory.rst Show resolved Hide resolved
machAeroTutorials/overset_theory.rst Outdated Show resolved Hide resolved
machAeroTutorials/overset_theory.rst Show resolved Hide resolved
machAeroTutorials/overset_surface_mesh.rst Show resolved Hide resolved
machAeroTutorials/overset_volume_mesh.rst Show resolved Hide resolved
machAeroTutorials/overset_analysis.rst Outdated Show resolved Hide resolved
ewu63
ewu63 previously approved these changes Mar 19, 2021
@anilyil anilyil marked this pull request as draft March 19, 2021 15:41
@anilyil
Copy link
Contributor

anilyil commented Mar 19, 2021

@DavidAnderegg I tried running the script and I thought they worked but they did not run. The mesh generation gets negative volumes on the tip section. The hole cutting also failed for me for both L2 and L3. Did not try L1 yet. Were the cases working for you when you created the PR?

@DavidAnderegg
Copy link
Contributor Author

@anilyil Strange. It did work.

Let me check, may be i switched something up?!?

@anilyil
Copy link
Contributor

anilyil commented Mar 19, 2021

I have a change in the volume mesh extrusion to fix the issue with the current surface meshes. But the repo as is did not run. I also realized I needed to update some adflow options for capitalization

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.

I agree with not delaying this PR, but I would like to make several changes to the overset theory. In particular, I would prefer to have Ney's text file be integrated into the rest of the page and remove duplicate and outdated information. Would it be better if I make an issue and address this in a separate PR?

@anilyil
Copy link
Contributor

anilyil commented Mar 19, 2021

Would it be better if I make an issue and address this in a separate PR?

I think you know what my preference will be there :)

@anilyil
Copy link
Contributor

anilyil commented Mar 19, 2021

Also the overset theory now has a whole bunch of debugging help etc. We can reorganize that in a separate PR imo, we can also address your point.

@DavidAnderegg
Copy link
Contributor Author

@anilyil I just tried aswell and it also failed. As the capitalization indicates, it was written for an older version of MACH-Aero. But I highly doubt this affected this. So i have actually no clue how that happend...

I also just checked the log files when i ran the simulations. ADflow does usually give a warning if there are negative volumes in the mesh, right? There are no warnings in the log I have.

On a general note, You dont have to hurry with this PR because of me. I never saw it as a high priority thing and I prefer it beeing helpfull an complete.

@sseraj sseraj mentioned this pull request Mar 19, 2021
@sseraj
Copy link
Collaborator

sseraj commented Mar 19, 2021

Also the overset theory now has a whole bunch of debugging help etc. We can reorganize that in a separate PR imo, we can also address your point.

I agree. I created issue #41. Feel free to add other to dos.

@anilyil
Copy link
Contributor

anilyil commented Mar 19, 2021

Yeah, I just want to get this done because it has been on my list for a while. Do you remember updating the surface mesh files at any point? I can update the runscript options but I doubt we made major changes to the math while you were working on this.

@DavidAnderegg
Copy link
Contributor Author

I am kicking into investigation mode. I just compared the output surface-meshes to the ones that lie in the repo. They are exactly the same.

It just came to my mind that when pyhyp was updated, some variables that could have been integers, now have to be float. Might that be the reason? I kinda doubt it, but that's the best i got at the moment...

@anilyil
Copy link
Contributor

anilyil commented Mar 19, 2021

@DavidAnderegg there is a block missing in the wing-tip mesh. its the one right near the tip on the TE. I could not see it in the volume extrusion. Its also not there on the surface mesh. You probably updated the surface mesh files at some point?

@DavidAnderegg
Copy link
Contributor Author

You are right. I did somehow not export this part, but in my analysis-files it is not missing. Strange. I will commit the proper surface-file.

But I also just noticed simpleOCart does apparently fail because the pyhyp options were changed (farfield must be lowercase now)

@anilyil
Copy link
Contributor

anilyil commented Mar 19, 2021

oh neat, I will update the pyhyp options and try again. the L3 mesh in the analysis folder did work with the overset

@sseraj
Copy link
Collaborator

sseraj commented Mar 19, 2021

But I also just noticed simpleOCart does apparently fail because the pyhyp options were changed (farfield must be lowercase now)

Is this with an old version of cgnsUtilities? I think I fixed this at the same time as the pyHyp update

@DavidAnderegg
Copy link
Contributor Author

But I also just noticed simpleOCart does apparently fail because the pyhyp options were changed (farfield must be lowercase now)

Is this with an old version of cgnsUtilities? I think I fixed this at the same time as the pyHyp update

This might very well be the case... I updated approx 2 months ago

@anilyil
Copy link
Contributor

anilyil commented Mar 19, 2021

I just updated all repos locally and things work. Will push an update in a second.

@anilyil anilyil marked this pull request as ready for review March 19, 2021 17:06
@anilyil
Copy link
Contributor

anilyil commented Mar 19, 2021

The meshes run and hole cutting script works now. I have created an issue on the mesh parameters but thats a secondary thing in my opinion. The issue is up at #42.

@joanibal do you want to review this? I dont want to put an extra burden on you. I think this is probably good to go for now but if you want to, feel free to take a look and leave comments.

@ewu63 ewu63 merged commit 0de6dcc into mdolab:master Mar 22, 2021
@ewu63
Copy link
Collaborator

ewu63 commented Mar 22, 2021

Thanks everyone and especially @DavidAnderegg for your contributions! 🎉 👏

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.

6 participants