Skip to content

fix axisym source term + update regression cases #1366

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.

Sign up for GitHub

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

Jump to bottom
Merged
FlorianDm merged 3 commits into from
Sep 2, 2021
Merged

fix axisym source term + update regression cases #1366

FlorianDm merged 3 commits into from
Sep 2, 2021

Conversation

@FlorianDm
Copy link
Member

@FlorianDm FlorianDm commented Sep 1, 2021

Proposed Changes

fix #1365

@FlorianDm FlorianDm mentioned this pull request Sep 1, 2021
Closed
@FlorianDm
Copy link
Member Author

FlorianDm commented Sep 1, 2021

I updated the restart file in the TestCases directory so I suppose regressions apart from this branch will break until this PR is merged. Sorry

@FlorianDm FlorianDm reopened this Sep 1, 2021
@pcarruscag
Copy link
Member

pcarruscag commented Sep 1, 2021

I have some vague memory of the textbook having a typo?

@WallyMaier
Copy link
Contributor

WallyMaier commented Sep 1, 2021 •
edited
Loading

@pcarruscag I also have a vague memory of that. But I couldn't seem to find it in the previous discussions....
@FlorianDm, could you also change NEMO_sources.cpp line 381 from:
+v*TWO3*(2*GV[nSpecies+2][1]-GV[nSpecies+2][0]
to
+v*TWO3*(2*GV[nSpecies+3][1]-GV[nSpecies+2][0]

@FlorianDm
Copy link
Member Author

FlorianDm commented Sep 1, 2021

Yes it did but it's unrelated. Surprised this bug here could creep in. For the regression it seems like it is still using the old restart file and not the one I pushed to TestCases master a few hours ago. Not sure what I am missing here

@FlorianDm
Copy link
Member Author

FlorianDm commented Sep 1, 2021

@WallyMaier OK will do

@WallyMaier
Copy link
Contributor

WallyMaier commented Sep 1, 2021

@FlorianDm, not sure the issue either. You could just do a "cold" start for the regression?

@pcarruscag
Copy link
Member

pcarruscag commented Sep 1, 2021

Don't push things to master directly
create a PR to develop and then merge, so we can keep track of things.
It's develop you need not master

@FlorianDm
Copy link
Member Author

FlorianDm commented Sep 1, 2021

Yes just realised that sorry
Will fix it

@FlorianDm FlorianDm reopened this Sep 1, 2021
FlorianDm added a commit to su2code/TestCases that referenced this pull request Sep 1, 2021
@FlorianDm
Merge pull request #75 from su2code/fix_axisym
d8b7434 
update restart file after bug fix su2code/SU2#1366
WallyMaier
WallyMaier approved these changes Sep 1, 2021
View reviewed changes
Copy link
Contributor

@WallyMaier WallyMaier 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 good to me! Im still surprised how this slipped through.

SU2_CFD/src/numerics/NEMO/NEMO_sources.cpp
-TWO3*AuxVar_Grad_i[0][1]);
residual[nSpecies+2] -= Volume*(yinv*(-sumJhs_y + total_viscosity_i*(u*(GV[nSpecies+3][0]+GV[nSpecies+2][1])
+v*TWO3*(2*GV[nSpecies+2][1]-GV[nSpecies+2][0]
+v*TWO3*(2*GV[nSpecies+3][1]-GV[nSpecies+2][0]
Copy link
Contributor

@WallyMaier WallyMaier Sep 1, 2021

Choose a reason for hiding this comment

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

Thanks!

@FlorianDm FlorianDm merged commit 63b2dea into develop Sep 2, 2021
@FlorianDm FlorianDm deleted the fix_axisym branch September 2, 2021 08:42
@pcarruscag pcarruscag mentioned this pull request Sep 19, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@WallyMaier WallyMaier WallyMaier approved these changes

Assignees

No one assigned

Labels

changelog:fix PR: merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@FlorianDm @pcarruscag @WallyMaier