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

[RomApplication] [Fast PR] Change the size allocation of phi_elemental and psi_elemental #12549

Merged
merged 1 commit into from
Jul 17, 2024

Conversation

Marco1410
Copy link
Contributor

📝 Description

This fast PR proposes to change the size allocation of phi_elemental and _psi_elemental from elem_dofs.size() to rhs_contribution.size().

This is because in transonic potential cases it is necessary to calculate the density at an upwind point for each element located in a supersonic region. Since the supersonic regions are not known a priori, and may vary during the calculation procedure, an upwind element is associated to each element mesh.

As a result, the finite element data structure needs to be enriched and here the elemental systems are extended (from number_of_degrees_of_freedom to number_of_degrees_of_freedom + 1) to contain additions to upwind nodes, only for those elements whose local mach number is greater than a certain critical_mach.

I think this is very specific for this case, but I don't think it will be a problem in a normal case and will broaden the use of the application.

🆕 Changelog

  • Updated rom_residuals_utility.h

@Marco1410 Marco1410 added FastPR This Pr is simple and / or has been already tested and the revision should be fast Refactor When code is moved or rewrote keeping the same behavior ROM labels Jul 17, 2024
@Marco1410 Marco1410 requested a review from a team as a code owner July 17, 2024 10:35
Copy link
Member

@rubenzorrilla rubenzorrilla left a comment

Choose a reason for hiding this comment

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

Makes sense for me. Nevertheless, I let the final approval to @Rbravo555 and @SADPR.

@Rbravo555
Copy link
Member

Rbravo555 commented Jul 17, 2024

This looks good to me as well. Just two comments:

  1. This bug will still be present in the ELEMENTAL BUILDER AND SOLVER, as the GetPhiElemental is fixed for all elements to the same size. Creating a similar fix should be done in another PR with medium priority.
  2. While launching all tests in the RomApplication with this branch, I came across two failing tests. However, they are not related to this change, but actually to missing .h5 files. HDF5 files are skipped due to the .gitignore. We should fix this with a high priority in another PR @NicolasSR, probably by saving the neural network parameters in another format not included in gitignore.

@Marco1410 Marco1410 merged commit 36aa2e7 into master Jul 17, 2024
9 of 11 checks passed
@Marco1410 Marco1410 deleted the RomApp/Change_allocation_size_phi_and_psi branch July 17, 2024 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FastPR This Pr is simple and / or has been already tested and the revision should be fast Refactor When code is moved or rewrote keeping the same behavior ROM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants