Skip to content

Epicure index fix#69

Open
amrueda wants to merge 11 commits into
developfrom
epicure_index_fix
Open

Epicure index fix#69
amrueda wants to merge 11 commits into
developfrom
epicure_index_fix

Conversation

@amrueda

@amrueda amrueda commented May 21, 2026

Copy link
Copy Markdown
Member

This PR fixes some index bugs in HexElementClass (cherry-picked from #58) and also enables anisotropic polynomial orders for conforming std DG discretizations.

@amrueda amrueda changed the base branch from main to develop May 21, 2026 13:05
@loganoz

loganoz commented May 21, 2026

Copy link
Copy Markdown
Collaborator

Pues no parece que funcione

@codecov

codecov Bot commented Jun 16, 2026

Copy link
Copy Markdown

Comment thread .github/workflows/CI_parallel_NS.yml Outdated
@amrueda

amrueda commented Jun 19, 2026

Copy link
Copy Markdown
Member Author

For some reason, the coverage tool is not doing a great job here, as it marks many lines as misses or partially covered, when they are actually exercised by the tests...

@amrueda amrueda requested a review from loganoz June 19, 2026 02:32
@Rodrigoansf

Copy link
Copy Markdown
Collaborator

That's great! I ran a battery of GPU tests with isotropic polynomial orders, including the full CI suite, and compared the runtimes against develop. I couldn't find a single test that runs slower with the index fix.

@loganoz

loganoz commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

@amrueda, I talked with @Rodrigoansf and it seems that all is working fine with this fix that you implemented. If you agree, I think we can merge this already. What do you think?

@amrueda

amrueda commented Jun 23, 2026

Copy link
Copy Markdown
Member Author

Great. Thank you guys!
If there is no objection, I think we can then proceed with the merge! :)

Copilot AI 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.

Pull request overview

This PR updates the mesh/discretization path to correctly support anisotropic polynomial orders (notably in volume weak integrals and element-to-face prolongation indexing), and updates/reenables the Navier–Stokes “CylinderDifferentOrders” regression test/CI to exercise the new behavior.

Changes:

  • Fix dimension/index usage in HexElementClass prolongation routines so face interpolation/prolongation uses the correct Nxyz component per face orientation.
  • Update DGIntegrals volume Green weak integral to handle anisotropic orders by separating derivative sums per direction.
  • Adjust the CylinderDifferentOrders test inputs/expected values and enable the test in the parallel NS CI workflow.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
Solver/test/NavierStokes/CylinderDifferentOrders/SETUP/ProblemFile.f90 Updates expected regression values and assertion style for residuals/forces.
Solver/test/NavierStokes/CylinderDifferentOrders/MESH/OrdersN2N1z.csv Adds an element-order file enabling anisotropic (2,2,1) orders.
Solver/test/NavierStokes/CylinderDifferentOrders/CylinderDifferentOrders.control Points the test to the new polynomial order file.
Solver/src/libs/mesh/HexElementClass.f90 Fixes face prolongation indexing for anisotropic Nxyz.
Solver/src/libs/mesh/FaceClass.f90 Adjusts Face_AdaptSolToFace dummy array shape for conforming anisotropic faces.
Solver/src/libs/discretization/DGIntegrals.f90 Corrects weak volume integral accumulation for anisotropic orders.
.github/workflows/CI_parallel_NS.yml Re-enables build/run steps for the CylinderDifferentOrders test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +572 to 575
CALL FTAssertEqual(expectedValue = residuals(1), &
actualValue = monitors % residuals % values(1,1), &
tol = 1.d-11, &
msg = "Continuity residual")
Comment on lines +612 to 615
CALL FTAssertEqual(expectedValue = cl, &
actualValue = monitors % surfaceMonitors(2) % values(1), &
tol = 1.d-11, &
msg = "Lift coefficient")
Comment on lines 351 to 355
type(Face), intent(inout) :: self
integer, intent(in) :: nEqn
integer, intent(in) :: Nelx, Nely
real(kind=RP), intent(in) :: Qe(1:NCONS, 0:Nelx, 0:Nely)
real(kind=RP), intent(in) :: Qe(1:NCONS, 0:self % NfRight(1), 0:self % NfRight(2))
integer, intent(in) :: side

@loganoz loganoz Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this fine? It seems a bit weird that Nelx and Nely are inputs to the subroutine but not used inside.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants