Skip to content

HLLD solver: Fix pressure scaling for low-Mach flows#1372

Merged
BenWibking merged 43 commits into
developmentfrom
nkriel-fix-HLLD-low-Mach-pressure-scaling
Dec 31, 2025
Merged

HLLD solver: Fix pressure scaling for low-Mach flows#1372
BenWibking merged 43 commits into
developmentfrom
nkriel-fix-HLLD-low-Mach-pressure-scaling

Conversation

@AstroKriel

@AstroKriel AstroKriel commented Oct 3, 2025

Copy link
Copy Markdown
Member

Description

This PR updates the HLLD solver to apply the same low-Mach pressure correction already implemented in the HLLC solver.

Related issues

Fixes the remaining issues detailed in #528

Checklist

  • I have added a description (see above).
  • I have added a link to any related issues (if applicable; see above).
  • I have read the Contributing Guide.
  • I have added tests for any new physics that this PR adds to the code.
  • (For quokka-astro org members) I have manually triggered the GPU tests with the magic comment /azp run.

@sonarqubecloud

sonarqubecloud Bot commented Oct 8, 2025

Copy link
Copy Markdown

@BenWibking BenWibking added the MHD label Oct 16, 2025
@sonarqubecloud

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread src/problems/HydroBalsaraVortex/test_hydro_balsara_vortex.cpp Outdated
Comment thread src/problems/MHDBalsaraVortex/testMHDBalsaraVortex.cpp Outdated
@AstroKriel

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@AstroKriel AstroKriel changed the title [WIP] HLLD solver: Fix pressure scaling for low-Mach flows HLLD solver: Fix pressure scaling for low-Mach flows Dec 28, 2025
@AstroKriel AstroKriel marked this pull request as ready for review December 28, 2025 01:22
@dosubot dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Dec 28, 2025
@BenWibking

Copy link
Copy Markdown
Collaborator

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the HLLD Riemann solver to include a pressure correction for low-Mach number flows, based on the work of Minoshima & Miyoshi (2021). The changes in src/hydro/HLLD.hpp appear to correctly implement the new formulation for the intermediate state pressure. A new test problem, MHDBalsaraVortex, is added to verify the changes, which is a good practice. However, I've found a potential issue in the analytical solution of this new test problem where the velocity and magnetic field perturbations seem to be incorrectly scaled by vortex_radius. This could lead to an incorrect error norm calculation if vortex_radius is not equal to 1. My review includes a suggestion to fix this.

Comment thread src/problems/MHDBalsaraVortex/testMHDBalsaraVortex.cpp
@BenWibking

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@sonarqubecloud

Copy link
Copy Markdown

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Dec 31, 2025
@BenWibking BenWibking added this pull request to the merge queue Dec 31, 2025
Merged via the queue into development with commit e0745b5 Dec 31, 2025
48 checks passed
@AstroKriel AstroKriel deleted the nkriel-fix-HLLD-low-Mach-pressure-scaling branch June 4, 2026 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer MHD priority:high high priority size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants