Skip to content

Conversation

@Giannis95
Copy link
Collaborator

closes #31612

@Giannis95 Giannis95 changed the title WIP : Add two MFEM auxkernels for L2-averaged inner and cross product of two source (par)gridfucntions projected onto an MFEM AuxVariable WIP: Add two MFEM auxkernels for L2-averaged inner and cross product of two source (par)gridfucntions projected onto an MFEM AuxVariable Sep 26, 2025
@GiudGiud
Copy link
Contributor

@Giannis95 we prefer rebasing (over devel usually, only over next if something you need is not in devel yet) over merge commits btw

Giannis95 pushed a commit to Giannis95/moose that referenced this pull request Oct 13, 2025
@GiudGiud GiudGiud self-assigned this Oct 15, 2025
@Giannis95 Giannis95 changed the title WIP: Add two MFEM auxkernels for L2-averaged inner and cross product of two source (par)gridfucntions projected onto an MFEM AuxVariable Add two MFEM auxkernels for L2-averaged inner and cross product of two source (par)gridfucntions projected onto an MFEM AuxVariable Oct 22, 2025
@Giannis95 Giannis95 marked this pull request as ready for review October 22, 2025 09:40
@Giannis95 Giannis95 requested a review from lindsayad as a code owner October 22, 2025 09:40
@moosebuild
Copy link
Contributor

Job Precheck, step Size check on 0e6176e wanted to post the following:

Warning: This PR changes repo size by 102.59 MiB.

@moosebuild
Copy link
Contributor

moosebuild commented Oct 22, 2025

Job Documentation, step Docs: sync website on c7d5559 wanted to post the following:

View the site here

This comment will be updated on new commits.

@Giannis95 Giannis95 force-pushed the Giannis95/MFEMInnerCrossProductAux branch from b141f31 to b5e1d6d Compare October 24, 2025 12:51
Copy link
Contributor

@GiudGiud GiudGiud left a comment

Choose a reason for hiding this comment

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

early review, nothing major

@GiudGiud
Copy link
Contributor

Seems one of the tests is really long btw. We should aim for <2s

please ping me when you want a review

@Giannis95
Copy link
Collaborator Author

Seems one of the tests is really long btw. We should aim for <2s

please ping me when you want a review

Hi, your previous comments are addressed, and this PR is now updated and ready for review. Thank you

@Giannis95 Giannis95 requested a review from GiudGiud November 3, 2025 15:03
…ce which will inncrease the test time (for the parallel test).Refs idaholab#31612
Copy link
Contributor

@GiudGiud GiudGiud left a comment

Choose a reason for hiding this comment

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

ready of that

I think you are missing a test for the postprocessor?

Co-authored-by: Guillaume Giudicelli <[email protected]>
@moosebuild
Copy link
Contributor

Job Precheck, step Clang format on 43e62dc wanted to post the following:

Your code requires style changes.

A patch was auto generated and copied here
You can directly apply the patch by running, in the top level of your repository:

curl -s https://mooseframework.inl.gov/docs/PRs/31613/clang_format/style.patch | git apply -v

Alternatively, with your repository up to date and in the top level of your repository:

git clang-format 7323e92e13d5c77e1d5af55f58c42cce19c9d9f4

@Giannis95
Copy link
Collaborator Author

Giannis95 commented Nov 6, 2025

ready of that

I think you are missing a test for the postprocessor?

I developed that postprocessor for the previous CSVdiff for the CrossProduct test, which had a runtime of >6s. Since we are now using a simpler test that does not need that postprocessor, it has been decided not to include it in this PR and to create another one at a later date, which will also update the terminology from _primal_var and dual_var to first and second in the other postprocessors too.

@moosebuild
Copy link
Contributor

Job Test, step Results summary on c7d5559 wanted to post the following:

Framework test summary

Compared against 7323e92 in job civet.inl.gov/job/3361666.

Removed tests

Added tests

Test Time (s)
mfem/auxkernels.MFEMDotProductAux.C 0.92
mfem/auxkernels.MFEMCrossProductAux.C 0.78

Run time changes

Modules test summary

Compared against 7323e92 in job civet.inl.gov/job/3361666.

Removed tests

Added tests

Run time changes

@moosebuild
Copy link
Contributor

Job Coverage, step Generate coverage on c7d5559 wanted to post the following:

Framework coverage

7323e9 #31613 c7d555
Total Total +/- New
Rate 86.02% 86.02% +0.00% 100.00%
Hits 124629 124696 +67 78
Misses 20254 20264 +10 0

Diff coverage report

Full coverage report

Modules coverage

Coverage did not change

Full coverage reports

Reports

This comment will be updated on new commits.

@GiudGiud
Copy link
Contributor

GiudGiud commented Nov 6, 2025

looks good

@GiudGiud GiudGiud merged commit 75e4c83 into idaholab:next Nov 6, 2025
68 of 69 checks passed
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.

Add two MFEM auxkernels for L2-averaged inner and cross product of two source (par)gridfucntions projected onto an MFEM AuxVariable

3 participants