-
Notifications
You must be signed in to change notification settings - Fork 108
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
Aachen Turbine Tutorial #56
base: develop
Are you sure you want to change the base?
Conversation
@joshkellyjak, could you review this PR? |
Hey Josh, could you have a look at this, so that we can finally close it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a copy of the test in TestCases?
Hi Pedro, the mesh is the same |
Then can we put this case in the tutorials.py regression and remove the one from TestCases? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree that this should be run as a regression test also
VISCOSITY_MODEL= SUTHERLAND | ||
% | ||
% Molecular Viscosity that would be constant (1.716E-5 by default) | ||
MU_CONSTANT= 1.716E-5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this does anything so maybe best to remove it to avoid confusing between this and MU_REF
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering that it works, I would avoid keep changing, just not to find out later that we missed something, as happened for the entropy fix in combination with JST.
% | ||
JST_SENSOR_COEFF= ( 0.5, 0.25 ) | ||
% Spatial numerical order integration | ||
MUSCL_FLOW= NO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove this option too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer keeping it, so that new users become aware of it.
Do you mean using this mesh in the tutorial files also for the regression test? If so, this change should not affect this PR. In case, could you guys give the final approve for this PR? |
Everything we put in tutorials must have a regression test in tutorials.py, no test, no approval. |
config and mesh files for the Aachen Turbine tutorial