Skip to content

Conversation

@joshkellyjak
Copy link
Contributor

Proposed Changes

The turbomachinery ramps were missing the option to ramp the translation rate in 2D and the ability to ramp the outlet mass flow, these have been added and the function has been made more general to accomodate these additions. It should be much easier to add ramps in the future.

Related Work

N/A

PR Checklist

Put an X by all that apply. You can fill this out after submitting the PR. If you have any questions, don't hesitate to ask! We want to help. These are a guide for you to know what the reviewers will be looking for in your contribution.

  • 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 used the pre-commit hook to prevent dirty commits and used pre-commit run --all to format old commits.
  • 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.

@joshkellyjak joshkellyjak changed the title Update turbo ramps [WIP] Update turbo ramps Jan 13, 2025
@joshkellyjak joshkellyjak changed the title [WIP] Update turbo ramps Update turbo ramps Jan 16, 2025
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.

Nice, just some minor stuff and it's good to go.

@joshkellyjak
Copy link
Contributor Author

Not entirely sure why reg tests are failing on this branch, I am unable to reproduce the same results seen here locally; my local results match the reg test values.

@pcarruscag
Copy link
Member

I was going to suggest compiler differences but the changes are huge

test_vals (stored): -10.467612, -2.871708, -19.345651, -13.625871, -11.582397, -6.306168, 73273.000000, 73273.000000, 0.019884, 82.491000
sim_vals (computed): -0.467189, 2.282461, -8.117118, -2.467358, -4.066032, 1.926555, 232280.000000, 232280.000000, -0.064599, -242.830000
delta_vals: 10.000423, 5.154169, 11.228533, 11.158513, 7.516365, 8.232723, 159007.000000, 159007.000000, 0.084483, 325.321000

Run valgrind...

@pcarruscag
Copy link
Member

Did you find anything with valgrind @joshkellyjak ?

@joshkellyjak
Copy link
Contributor Author

Did you find anything with valgrind @joshkellyjak ?

If I remember correctly nothing turned up. Will run again today and double check.

@joshkellyjak
Copy link
Contributor Author

Nothing turning up in Valgrind. In fact I am still unable to reproduce the discrepency seen in the regression tests locally, the values match perfectly everywhere but here, further none of the regression tests should even be effected by the modifications in this PR (I guess this an issue in itself). Any thoughts @pcarruscag ?

@pcarruscag
Copy link
Member

What's all this then? 🤣

==20496== Conditional jump or move depends on uninitialised value(s)
==20496==    at 0x970BD3: CConfig::SetPostprocessing(SU2_COMPONENT, unsigned short, unsigned short) (CConfig.cpp:4427)
==20496==    by 0x98E830: CConfig::CConfig(char*, SU2_COMPONENT) (CConfig.cpp:219)
==20496==    by 0x232AD2: main (SU2_CFD.cpp:87)
==20496== 
==20496== Conditional jump or move depends on uninitialised value(s)
==20496==    at 0x970BE0: CConfig::SetPostprocessing(SU2_COMPONENT, unsigned short, unsigned short) (CConfig.cpp:4434)
==20496==    by 0x98E830: CConfig::CConfig(char*, SU2_COMPONENT) (CConfig.cpp:219)
==20496==    by 0x232AD2: main (SU2_CFD.cpp:87)
==20496== 
==20496== Conditional jump or move depends on uninitialised value(s)
==20496==    at 0x970BF6: CConfig::SetPostprocessing(SU2_COMPONENT, unsigned short, unsigned short) (CConfig.cpp:4441)
==20496==    by 0x98E830: CConfig::CConfig(char*, SU2_COMPONENT) (CConfig.cpp:219)
==20496==    by 0x232AD2: main (SU2_CFD.cpp:87)
==20496== 
==20496== Conditional jump or move depends on uninitialised value(s)
==20496==    at 0x9713C7: CConfig::SetPostprocessing(SU2_COMPONENT, unsigned short, unsigned short) (CConfig.cpp:4456)
==20496==    by 0x98E830: CConfig::CConfig(char*, SU2_COMPONENT) (CConfig.cpp:219)
==20496==    by 0x232AD2: main (SU2_CFD.cpp:87)
==20496== 
image

@joshkellyjak
Copy link
Contributor Author

What's all this then? 🤣

==20496== Conditional jump or move depends on uninitialised value(s)
==20496==    at 0x970BD3: CConfig::SetPostprocessing(SU2_COMPONENT, unsigned short, unsigned short) (CConfig.cpp:4427)
==20496==    by 0x98E830: CConfig::CConfig(char*, SU2_COMPONENT) (CConfig.cpp:219)
==20496==    by 0x232AD2: main (SU2_CFD.cpp:87)
==20496== 
==20496== Conditional jump or move depends on uninitialised value(s)
==20496==    at 0x970BE0: CConfig::SetPostprocessing(SU2_COMPONENT, unsigned short, unsigned short) (CConfig.cpp:4434)
==20496==    by 0x98E830: CConfig::CConfig(char*, SU2_COMPONENT) (CConfig.cpp:219)
==20496==    by 0x232AD2: main (SU2_CFD.cpp:87)
==20496== 
==20496== Conditional jump or move depends on uninitialised value(s)
==20496==    at 0x970BF6: CConfig::SetPostprocessing(SU2_COMPONENT, unsigned short, unsigned short) (CConfig.cpp:4441)
==20496==    by 0x98E830: CConfig::CConfig(char*, SU2_COMPONENT) (CConfig.cpp:219)
==20496==    by 0x232AD2: main (SU2_CFD.cpp:87)
==20496== 
==20496== Conditional jump or move depends on uninitialised value(s)
==20496==    at 0x9713C7: CConfig::SetPostprocessing(SU2_COMPONENT, unsigned short, unsigned short) (CConfig.cpp:4456)
==20496==    by 0x98E830: CConfig::CConfig(char*, SU2_COMPONENT) (CConfig.cpp:219)
==20496==    by 0x232AD2: main (SU2_CFD.cpp:87)
==20496== 
image

Oh bugger, I must've missed out a valgrind option. Cheers, I'll try and find some time to fix this evening.

@pcarruscag
Copy link
Member

You may want to try different build options, debug vs release. I'm on GCC 13 and valgrind picked up those issues.
If it helps, I think some of those CConfig functions you are calling depend on variables that are only set further down in the config processing.

@joshkellyjak
Copy link
Contributor Author

I am using debugoptimized, I will give the others a go. I am also on gcc-13 but I did update recently, I can't remember what I was on when I initially did the test. I think I was missing the verbosity option, running a quick test again now and the error is displayed when this is enabled in valgrind, and doesn't when it is not.

Yes I think I see the issue, the bool values are set false after these erroneous lines when the ramp is not engaged. Should be a pretty straightforward fix.

@joshkellyjak
Copy link
Contributor Author

Updated testcases and everything looks good now. Once su2code/Tutorials#66 is merged I will update workflows and this is ready to go.

@pcarruscag pcarruscag merged commit 4e9fa0a into develop Sep 1, 2025
37 checks passed
@pcarruscag pcarruscag deleted the feature_turbo_ramps branch September 1, 2025 01:59
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.

5 participants