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

Add Velocity volume output for compressible flow solvers #2077

Merged
merged 12 commits into from
Aug 7, 2023

Conversation

MastermindScope
Copy link
Contributor

@MastermindScope MastermindScope commented Jul 10, 2023

Disclaimer

This is my first ever contribution to any GitHub project. Please give feedback on what I can Improve if I haven't done something that is expected of me.

Proposed Changes

Added Velocity output, since I am interested in that and adding it only in the config just gets ignored and no output happens

Related Work

PR Checklist

  • I am submitting my contribution to the develop branch.
  • My contribution generates no new compiler warnings (try with --warnlevel=3 when using meson).
  • My contribution is commented and consistent with SU2 style (https://su2code.github.io/docs_v7/Style-Guide/).
  • I have added a test case that demonstrates my contribution, if necessary.
  • I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp), if necessary.

Sorry, something went wrong.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Added Velocity output, since I am interested in that and adding it only in the config just gets ignored and no output happens
Copy link
Member

@pcarruscag pcarruscag left a comment

Choose a reason for hiding this comment

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

Welcome to SU2. You need to add these variables to the "PRIMITIVE" group, there is a comment // Primitive variables.
Look also for SetVolumeOutputValue("PRESSURE" because for each variable you need to add it and then set its value.
Please make the same change in CNEMOCompOutput.cpp
And add yourself to AUTHORS.md

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Added Velocity Volume Output

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Added Velocity Output

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@MastermindScope
Copy link
Contributor Author

Hi, thank you for the warm welcome. I added the changes and hope they are sufficient.

@MastermindScope MastermindScope changed the title Add Velocity Volume Output To CFlowCompOutput.cpp Add Velocity Volume Output To CFlowCompOutput.cpp and CNEMOCompOutput.cpp Jul 11, 2023
Copy link
Member

@pcarruscag pcarruscag left a comment

Choose a reason for hiding this comment

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

Thanks, you are still missing the part of the code that sets the output value, AddVolumeOutput only adds the name.
Look for SetVolumeOutputValue in the files you changed, should be very close.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@pcarruscag pcarruscag changed the base branch from master to develop July 11, 2023 15:53
pcarruscag and others added 3 commits July 11, 2023 08:53

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@MastermindScope
Copy link
Contributor Author

I've added SetVolumeOutputValue now. Thank you for being patient and letting me make mistakes.

@EvertBunschoten
Copy link
Member

Hi! Just out of curiosity, if you're interested in just the velocity and not the momentum, why not just divide the momentum vector by the density during postprocessing?

Copy link
Member

@EvertBunschoten EvertBunschoten left a comment

Choose a reason for hiding this comment

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

Make sure you retrieve the velocity from the Primitive vector stored in the CVariable classes, not the Solution vector.

@MastermindScope
Copy link
Contributor Author

why not just divide the momentum vector by the density during postprocessing?

There are several reasons why:

  1. I thought of this, but it looks patchy somehow
  2. It makes life easier for people who want to use this software but have little experience (like me)
  3. I want to start contributing to this project and this was one of the things I noticed and thought I could implement myself

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…WallyMaier
@pcarruscag pcarruscag changed the title Add Velocity Volume Output To CFlowCompOutput.cpp and CNEMOCompOutput.cpp Add Velocity volume output for compressible flow solvers Jul 17, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@pcarruscag pcarruscag merged commit aca53d5 into su2code:develop Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants