Skip to content

Conversation

@EvertBunschoten
Copy link
Member

@EvertBunschoten EvertBunschoten commented Nov 21, 2022

Proposed Changes

Generalize the variables in the CLookUpTable class for use in general look-up cases (not only progress variable-enthalpy).
Add dimension to CLookUpTable class to allow for quasi-3D interpolation (linear interpolation between two trapezoidal maps stacked in the third dimension).

I added a unit test as a test case with the quasi-3D table to show the correct use of syntax for 3D look-up operations.

Related Work

Can be used in feature_multilayer_perceptron in the CDataDrivenFluid fluid model for look-up operations.

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 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 pcarruscag changed the base branch from master to develop November 21, 2022 18:24
…as they're redundant. Added a 3D table unit test case
@su2code su2code deleted a comment from lgtm-com bot Dec 13, 2022
@su2code su2code deleted a comment from lgtm-com bot Dec 13, 2022
@su2code su2code deleted a comment from lgtm-com bot Dec 13, 2022
@su2code su2code deleted a comment from lgtm-com bot Dec 13, 2022
@su2code su2code deleted a comment from lgtm-com bot Dec 13, 2022
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.

Anything else on your side @bigfooted ?

@bigfooted
Copy link
Contributor

Anything else on your side @bigfooted ?

@pcarruscag
I had some small comments, besides that it looks good.
@EvertBunschoten
We can look into generalizing and merging the 2D and 3D LUT approach once the flamelet model is also in develop, but this also means changing the LUT file format. There are some open issues like how to best treat lookups that fall outside of the table, and the extention from the slab approach to the trapezoidal map approach would definitely help with the memory consumption. Not a show stopper right now, but would be nice to do this as part of a student project this year.

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.

Ok you can figure out the order you want to merge things.
LGTM

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.

LGTM

@pr-triage pr-triage bot removed the PR: unreviewed label Feb 6, 2023
@EvertBunschoten EvertBunschoten merged commit 8aa27dc into develop Feb 7, 2023
@EvertBunschoten EvertBunschoten deleted the feature_general_lookup_table branch February 7, 2023 06:39
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