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

Improved look-up table query scaling with variable count #2214

Merged
merged 9 commits into from
Feb 26, 2024

Conversation

EvertBunschoten
Copy link
Member

@EvertBunschoten EvertBunschoten commented Feb 16, 2024

Proposed Changes

The CLookUpTable class allows for the fast interpolation of unstructured 2D data.

It was found that the query time of this class scaled significantly with the number of output variables, indicating inefficiencies in the query process.

These inefficiencies came from string comparison routines in the GetIndexOfVar function of the CLookUpTable class. By implementing overloaded instances of the look-up functions which retrieve data from data indices directly, the query time efficiency scaling was significantly improved.

Also reduced duplicate code within the CLookUpTable class.

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.

@pcarruscag
Copy link
Member

You may want to remove the string based interface since it's known to be inneficient.

@EvertBunschoten
Copy link
Member Author

You may want to remove the string based interface since it's known to be inneficient.

When replacing the string-based with index-based look-up methods, the performance improves substantially. The figure below shows the average measured evaluation time (measured with chrono library) vs the number of output variables. It's clear that using index-based look-up methods results in reduced query time, as well as improved scaling.

scaling_nVars

Common/src/containers/CLookUpTable.cpp Fixed Show fixed Hide fixed
Common/src/containers/CLookUpTable.cpp Fixed Show fixed Hide fixed
Common/src/containers/CLookUpTable.cpp Fixed Show fixed Hide fixed
Common/src/containers/CLookUpTable.cpp Fixed Show fixed Hide fixed
Common/src/containers/CLookUpTable.cpp Fixed Show fixed Hide fixed
Common/src/containers/CLookUpTable.cpp Fixed Show fixed Hide fixed
Common/src/containers/CLookUpTable.cpp Fixed Show fixed Hide fixed
@EvertBunschoten EvertBunschoten marked this pull request as ready for review February 23, 2024 13:10
@EvertBunschoten EvertBunschoten changed the title [WIP] Improved look-up table query scaling with variable count Improved look-up table query scaling with variable count Feb 23, 2024
Copy link
Contributor

@bigfooted bigfooted left a comment

Choose a reason for hiding this comment

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

Thanks Evert, it looks good to me, Also nice improvement in speed.

SU2_CFD/src/fluid/CFluidFlamelet.cpp Outdated Show resolved Hide resolved
Common/include/containers/CLookUpTable.hpp Outdated Show resolved Hide resolved
@EvertBunschoten EvertBunschoten merged commit c1acf46 into develop Feb 26, 2024
31 checks passed
@EvertBunschoten EvertBunschoten deleted the fix_lookup_table_scaling branch February 26, 2024 11:18
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.

3 participants