Skip to content

Port some Hermes-3 finite volume operators (rebased)#3335

Merged
dschwoerer merged 51 commits into
nextfrom
move-ops
Jul 6, 2026
Merged

Port some Hermes-3 finite volume operators (rebased)#3335
dschwoerer merged 51 commits into
nextfrom
move-ops

Conversation

@dschwoerer

Copy link
Copy Markdown
Contributor

Rebase of #3200 on top of #3320 and #3334

@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 include/bout/deriv_store.hxx
Comment thread include/bout/deriv_store.hxx
Comment thread include/bout/deriv_store.hxx
Comment thread include/bout/field3d.hxx Outdated
Comment thread include/bout/field3d.hxx
Comment thread src/mesh/difops.cxx
Comment thread src/mesh/parallel/fci.cxx Outdated
Comment thread src/mesh/parallel/fci.cxx
Comment thread src/mesh/parallel/fci.cxx Outdated
Comment thread src/sys/derivs.cxx
@ZedThree

Copy link
Copy Markdown
Member

A couple of points that haven't been done:

  • @mrhardman pointed out some other operators that could potentially be ported, along with their checks Port some Hermes-3 finite volume operators #3200 (comment)
  • there's now a lot of code in the fv_ops.hxx header -- if the set of possible CellEdges implementations is closed to what's in BOUT++, it would be possible to shift stuff out of the header and provide explicit instantiations of the template functions. This would probably speed up compilation, at the cost of being harder to add new limiters

@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/mesh/difops.cxx Outdated
Comment thread src/mesh/difops.cxx
Comment thread tests/MMS/spatial/finite-volume/fv_mms.cxx
@dschwoerer

Copy link
Copy Markdown
Contributor Author

A couple of points that haven't been done:

@mrhardman pointed out some other operators that could potentially be ported, along with their checks Port some Hermes-3 finite volume operators #3200 (comment)

Sorry, I tried to follow to the comment, and from there to two PRs, but I failed to see what operators you would like to add here?
Do you also want to add them to these tests, or do you want to also port some of the non-orthogonal mesh tests to BOUT++?
I am not objecting to that, but could this be a separate PR?
This one already "blocks" #3392 and #3405.

there's now a lot of code in the fv_ops.hxx header -- if the set of possible CellEdges implementations is closed to what's in BOUT++, it would be possible to shift stuff out of the header and provide explicit instantiations of the template functions. This would probably speed up compilation, at the cost of being harder to add new limiters

✔️ done in #3405

@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 include/bout/fv_ops.hxx
Comment thread include/bout/fv_ops.hxx Outdated
@dschwoerer

Copy link
Copy Markdown
Contributor Author

I found some MMS tests in hermes-3 that fail with this PR. I will have look whether they can be moved to BOUT++ and fixed.

@dschwoerer dschwoerer marked this pull request as draft June 30, 2026 07:07
We need to ensure that fv_ops_impl.hxx is never parsed before
fv_ops.hxx, as fv_ops.hxx provides default arguments, which is
forbiddef for a redeclaration. To ensure this, fv_ops.hxx is included
in fv_ops_impl.hxx, so that fv_ops_impl.hxx can be included and
fv_ops.hxx can appear afterwards without causing an error.
@dschwoerer

Copy link
Copy Markdown
Contributor Author

The MMS tests should be fixed by f893f3d

CI run here: https://github.com/boutproject/hermes-3/actions/runs/28581902968

@dschwoerer dschwoerer marked this pull request as ready for review July 2, 2026 10:08

@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 include/bout/fv_ops_impl.hxx
bendudson added 3 commits July 4, 2026 08:54
Was testing _cell_volume.has_value() rather than
_cell_area_zlow.has_value().

Added regression test.
Field-aligned operation returns a flow diagnostic, but this is
not currently done for FCI. Set to zero and document behavior.
Short descriptions of the ported operators, SuperBee slope limiter,
and metric derived quantity caching.

@bendudson bendudson 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.

Thanks @dschwoerer !

@dschwoerer dschwoerer merged commit e5c3a61 into next Jul 6, 2026
23 checks passed
@dschwoerer dschwoerer deleted the move-ops branch July 6, 2026 12:33
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