Skip to content

Nonorthogonal with minimal differential operators MMS test.#285

Merged
mrhardman merged 150 commits into
masterfrom
nonorthogonal-divops-mms-test-minimal
Jan 29, 2026
Merged

Nonorthogonal with minimal differential operators MMS test.#285
mrhardman merged 150 commits into
masterfrom
nonorthogonal-divops-mms-test-minimal

Conversation

@mrhardman
Copy link
Copy Markdown
Collaborator

@mrhardman mrhardman commented Jan 24, 2025

Fixes to the Div_a_Grad_perp_nonorthog(a, f) operator (commit 3d11cb7, commit 2e5e827), merge with master, and new automatic tests integrated with ctest showing a correct implementation. Theses tests are hermes-3/tests/mms/orthogonal_test.py and hermes-3/tests/mms/nonorthogonal_test.py, and may be run interactively by, e.g.,

cd hermes-3/build-path/tests/mms/
python3 orthogonal_test.py

or

cd hermes-3/build-path/
ctest -I 12

These tests can be readily extended to test new nonorthogonal operators and the other existing operators in the Hermes-3 code base.

See /hermes-3/tests/mmsREADME.md for some documentation. Comments and requests appreciated.

Thanks to @dschwoerer for providing an initial MMS test to develop. Thanks to @johnomotani for numerous suggestions and bugfixes.

Below is the plot output from the tests, showing the error (blue), the expected (orange), and the fit of the error (green). Increase the range of $\Delta$ by increasing ntest in the test_input dictionary in hermes-3/tests/mms/orthogonal_test.py and hermes-3/tests/mms/nonorthogonal_test.py. Note that since the number of grid points $N_x = N_y = N_z$, the cost of the test rapidly increases with ntest.

FV::Div_a_Grad_perp(a, f) with orthogonal metric
fig_0
Div_a_Grad_perp_nonorthog(a, f) with orthogonal metric
fig_1
Div_a_Grad_perp_nonorthog(a, f) with nonorthogonal metric
fig_0_non

bendudson and others added 30 commits November 28, 2022 22:54
A version of FV::Div_a_Grad_perp that includes all metric components,
including g12 (X-Y non-orthogonal) and g13 (X-Z nonorthogonal) terms.

Aim to test and then merge upstream into BOUT++
Testing, using operator that includes nonorthogonal mesh corrections
…ile as this causes problems when loading from xbout.
… of convergence for orthogonal grid operator with a simple metric constant in x, y, z.
Copy link
Copy Markdown
Collaborator

@mikekryjak mikekryjak left a comment

Choose a reason for hiding this comment

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

Looks good to me apart from two minor points:

  • Let's rename the mms folder to mms-operators since there are other mms tests in other folders.
  • rnn_override could use a better name.

Comment thread src/neutral_mixed.cxx Outdated
@dschwoerer
Copy link
Copy Markdown
Collaborator

There a lot of revert commits in this PR. Would it make sense to git rebase this, and remove the initial commits and the reverts?

Did understand it correctly that this PR, which adds new tests, should not be merged, as the unit test coverage is decreasing?

@mikekryjak
Copy link
Copy Markdown
Collaborator

Did understand it correctly that this PR, which adds new tests, should not be merged, as the unit test coverage is decreasing?

I think we can make an exception since this is considerably adding to our overall test capability.

@mrhardman
Copy link
Copy Markdown
Collaborator Author

Did understand it correctly that this PR, which adds new tests, should not be merged, as the unit test coverage is decreasing?

If you can suggest unit tests in the C++ files affected, I am willing to try to implement them. However, as pointed out, this PR mostly adds tests, which are worthwhile in of themselves, even if code cov does not count them.

…mms/*` to `test/mms_operator/*`. This name change is motivated by the observation that `tests/integrated` contains some tests using the method of manufactured solutions (MMS).
@mrhardman
Copy link
Copy Markdown
Collaborator Author

@mikekryjak Your comments should now be addressed by e97c363 and 73c8879.

@mikekryjak mikekryjak mentioned this pull request Jan 19, 2026
…ure.cxx or evolve_density.cxx. Introduce a similar set of unit tests. The integration tests fail: the changes have unexpectedly broken the component.
@mikekryjak
Copy link
Copy Markdown
Collaborator

@mikekryjak Your comments should now be addressed by e97c363 and 73c8879.

Looks great, I'm happy for this to be merged in.

@mrhardman
Copy link
Copy Markdown
Collaborator Author

@mikekryjak Your comments should now be addressed by e97c363 and 73c8879.

Looks great, I'm happy for this to be merged in.

I will merge master to resolve the conflicts. There are many commits in this PR. Would you be happy for me to squash the commits in the merge?

@mrhardman mrhardman requested a review from mikekryjak January 29, 2026 12:46
@mrhardman mrhardman merged commit f6ae2d1 into master Jan 29, 2026
1 check passed
@mrhardman mrhardman deleted the nonorthogonal-divops-mms-test-minimal branch January 29, 2026 16:53
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.

6 participants