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

[RomApp] Take into account non converged solutions in Linear and NonLinear (ANN-Enhanced) Decoders #12572

Merged
merged 3 commits into from
Jul 24, 2024

Conversation

Rbravo555
Copy link
Member

📝 Description
This PR allows to take into account the nonconverged solutions when this parameters is true

general_rom_manager_parameters["ROM"]["use_non_converged_sols"].GetBool()

In future PRs we should include other types of SVDs, e.g. partitioned or parallel methods, to be used with the RomManager for cases where matrices are too large to fit in fast memory.

🆕 Changelog

  • Changed method _LaunchComputeSolutionBasis in RomManager to consider nonconversed solutions
  • Changed _GetTrainingData method in rom_nn_trainer.py to simplify the inclusion of nonconverged solutions while training a neural network
  • Fixed a typo in the RomManager

@Rbravo555 Rbravo555 added the ROM label Jul 23, 2024
@Rbravo555 Rbravo555 requested a review from a team as a code owner July 23, 2024 10:51
Copy link
Contributor

@NicolasSR NicolasSR left a comment

Choose a reason for hiding this comment

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

Checked the code and tested it. Everything looks fine.
As an idea for the future, we could limit the number of non-converged samples to store, by just skipping every n'th step or something more sophisticated

@Rbravo555 Rbravo555 merged commit 87a4c4e into master Jul 24, 2024
9 of 11 checks passed
@Rbravo555 Rbravo555 deleted the RomApp_TakeIntoAccountNonConvergedSols branch July 24, 2024 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

3 participants