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

Feature multilayer perceptron and data-driven fluid model #1757

Merged
merged 146 commits into from
Jul 5, 2023

Conversation

EvertBunschoten
Copy link
Member

@EvertBunschoten EvertBunschoten commented Sep 13, 2022

Proposed Changes

Addition of multi-layer perceptron class which can be used to evaluate trained multi-layer perceptrons in processes such as thermodynamic state evaluation in data-driven fluid models.

Related Work

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.

In the Tutorials repository, branch feature_multilayer_perceptron, one can find under "common/multilayer_perceptron" some python scripts which demonstrate how to translate a model trained through Tensorflow to an input file compatible with the CLookUp_ANN class. The README.md file in that tutorial folder also lists some important information regarding the functionality of the CLookUp_ANN class, datadriven fluid model, and MLP input file format.

Shall I leave those there or should these be uploaded elsewhere?

  • 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.

@pr-triage pr-triage bot added the PR: draft label Sep 13, 2022
@lgtm-com
Copy link

lgtm-com bot commented Sep 13, 2022

This pull request introduces 6 alerts when merging 9370536 into 45214cd - view on LGTM.com

new alerts:

  • 4 for Resource not released in destructor
  • 1 for Non-virtual destructor in base class
  • 1 for Constant return type on member

@bigfooted bigfooted changed the title Feature multilayer perceptron [WIP] Feature multilayer perceptron Sep 13, 2022
@pcarruscag
Copy link
Member

Do you want to introduce this first as a standalone library, and then start using it for fluid models?

@EvertBunschoten
Copy link
Member Author

@pcarruscag Yes I want to introduce the multilayer perceptron as an option for users and developers for things like data-driven fluid models. Do you think it would be good to add a template CFluidModel child class demonstrating how the MLP class can be used to create data-driven fluid models (apart from writing a tutorial of course)?

@pcarruscag
Copy link
Member

Sounds good. Initially it would be enough to have some unit tests that would already show how to setup the network, together with documentation / example of the file format. Applications can come after.

Some initial comments:
Please move the files to toolboxes/ and ideally use a namespace for the new classes.
Start the class names with C as we do, e.g. CIOMap.

Then, how large are the models you've used so far? and how important is performance to this feature? (Just so I know how much to comment on that)

@EvertBunschoten
Copy link
Member Author

Of course I'll provide an example of an MLP input file, as well as a python script I wrote that translates an MLP trained through TensorFlow to such an input file.

Very well, I'll move the files from the numerics folder to the common folder and change names accordingly.

The models I used so far had between 5 and 50 perceptrons and up to 15 layers. Performance is quite important, as evaluation of MLP's is generally more computationally expensive than for example lookup tables. The larger the MLP architecture, the more costly evaluations get of course. Any improvement to the computation speed will therefore be welcome. In terms of memory, the MLP's don't seem to be an issue so far.

@lgtm-com
Copy link

lgtm-com bot commented Sep 15, 2022

This pull request introduces 3 alerts when merging 47b456c into 88c8392 - view on LGTM.com

new alerts:

  • 3 for Resource not released in destructor

@lgtm-com
Copy link

lgtm-com bot commented Sep 26, 2022

This pull request introduces 3 alerts when merging 88f0012 into d32ccec - view on LGTM.com

new alerts:

  • 3 for Resource not released in destructor

@lgtm-com
Copy link

lgtm-com bot commented Oct 19, 2022

This pull request introduces 3 alerts when merging 04813b1 into 124795b - view on LGTM.com

new alerts:

  • 3 for Resource not released in destructor

@lgtm-com
Copy link

lgtm-com bot commented Nov 18, 2022

This pull request introduces 3 alerts when merging 112876d into 7132d99 - view on LGTM.com

new alerts:

  • 3 for Resource not released in destructor

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

@lgtm-com
Copy link

lgtm-com bot commented Nov 21, 2022

This pull request introduces 3 alerts when merging e8a3a93 into 7132d99 - view on LGTM.com

new alerts:

  • 3 for Resource not released in destructor

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

@EvertBunschoten EvertBunschoten changed the title Feature multilayer perceptron and data-driven fluid model [WIP]Feature multilayer perceptron and data-driven fluid model Jun 26, 2023
@EvertBunschoten EvertBunschoten marked this pull request as draft June 26, 2023 10:12
… that initial values for density and energy no longer need to be provided by the user
@EvertBunschoten EvertBunschoten changed the title [WIP]Feature multilayer perceptron and data-driven fluid model Feature multilayer perceptron and data-driven fluid model Jun 27, 2023
@bigfooted
Copy link
Contributor

@EvertBunschoten I suggest to first finish and close this one before moving on to the hydrogen flamelets PR

@EvertBunschoten EvertBunschoten marked this pull request as ready for review July 4, 2023 12:53
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.

I'll ignore the request for review the same way you ignored my questions 🤷

@EvertBunschoten
Copy link
Member Author

I'll ignore the request for review the same way you ignored my questions shrug

Could you clarify your questions? I sent you a message on Slack.

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.

Evert thought he was replying but the messages were pending and never made it online.

@EvertBunschoten EvertBunschoten merged commit 2a0fd69 into develop Jul 5, 2023
@EvertBunschoten EvertBunschoten deleted the feature_multilayer_perceptron branch July 5, 2023 20:26
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.

4 participants