[WIP] Consolidate sheath boundaries #382

ZedThree · 2025-08-05T15:18:23Z

Includes #368. ~~e94eb10 is the commit merging sheath_boundary into sheath_boundary_simple.~~

~~Current task is to see about merging sheath_boundary_insulating in as well.~~

Merges sheath_boundary, sheath_boundary_simple, and sheath_boundary_insulating into a single implementation with compile-time switches between the different conditions.

Is SheathKind::normal a sensible name for the "non-simple" case? (Note that this is an internal name only, and not user-visible).

TODO:

guard input options that aren't appropriate for the different sheath kinds

`phi` must be calculated in field-aligned coordinates, and transformed to unaligned

Lower-y loop used wrong cell face

Possibly faster, certainly cleaner

Didn't flip noflow BCs at same time, so was not exactly symmetric

src/sheath_boundary_simple.cxx

ZedThree · 2025-08-05T15:21:06Z

src/sheath_boundary_simple.cxx

+          const BoutReal _gamma_i = 2.5 + (0.5 * Mi * C_i_sq / tisheath);
+
+          const auto _q =
+              ((_gamma_i - 1 - 1 / (adiabatic - 1)) * tisheath - 0.5 * Mi * C_i_sq)
+              * nisheath * visheath;


This is different from the form of gamma_e. Notably there's an explicit 2.5 here. Is this correct?

Hmmm..... I don't know of any reason why the gamma expression should be any different than in sheath_boundary_simple, apart from that sheath_boundary had it hardcoded. @bendudson needs to confirm

mikekryjak · 2025-08-06T09:07:29Z

sheath_boundary specifically allows multiple ion species, so I would maybe call it SheathKind::multispecies.

ZedThree · 2025-08-08T08:49:45Z

sheath_boundary specifically allows multiple ion species, so I would maybe call it SheathKind::multispecies.

The simple version also works over multiple ion species, so I'm not sure that's a useful distinguishing feature!

* master: (54 commits) Silence some compiler warnings. Increase the test tolerance to work around compiler differences (seen with -O0 vs. -O2 and gcc@12+). fixed_velocity: Add description of mesh array to manual Typo in assert message. fixed_velocity: Read velocity from mesh Extend test input densities, temperatures outside clipped ranges. ADAS CX tests. Use common input ranges for all reaction regression tests. Rename test base class for ADAS izn/rec. Move AmjuelCXTest::generate_state() up to a parent class, so it can reused for ADAS. Rejig ReactionTest::sources_regression_test to check species data stored in (sub-)sections, including collision freqencies. evolve_pressure: Initialize flow_ylow if diagnose but no velocity Add H isotope CX tests. Move generate_data() for ADAS, Amjuel izn and rec tests into a separate class. Consistent naming of reaction regression data files. ADAS ionisation, recombination reaction regression tests. Correct a mistake in the docs Makefile that meant the doxygen target wasn't declared as .PHONY. Tidy up ReactionTest::generate_state(). Revert "Rename doxygen Makefile target to avoid clash with docs subdirectory name." Add missing header. ...

Add to loosen tolerance on sheath test slightly as higher `Ge` test has smaller `Te,Ve` so larger relative error. Difference between old and new implementation does appear to be from build up of numerical errors rather than anything systematic. New test at intermediate `Ge` to show this.

mikekryjak · 2025-08-08T15:00:48Z

sheath_boundary specifically allows multiple ion species, so I would maybe call it SheathKind::multispecies.

The simple version also works over multiple ion species, so I'm not sure that's a useful distinguishing feature!

Well yes, this is technically true, but sheath_boundary_simple is not recommended for multiple ion species due to the lack of proper accounting in the heat transmission coefficients.

ZedThree · 2025-08-08T15:03:36Z

Well yes, this is technically true, but sheath_boundary_simple is not recommended for multiple ion species due to the lack of proper accounting in the heat transmission coefficients.

Ah, maybe it would be wise to raise a warning or something in that case? That's a separate issue I suppose

…s-db merge main into consolidate-sheath-boundaries

dschwoerer · 2025-10-07T08:14:16Z

src/sheath_boundary.cxx

+      const BoutReal Ni_im = limitFree(Ni[ip], Ni[i], density_boundary_mode);
+      const BoutReal Ti_im = limitFree(Ti[ip], Ti[i], temperature_boundary_mode);
+      const BoutReal Te_im = limitFree(Te[ip], Te[i], temperature_boundary_mode);
+
+      // Calculate sheath values at half-way points (cell edge)
+      const BoutReal nisheath = 0.5 * (Ni_im + Ni[i]);
+      // electron temperature
+      const BoutReal tesheath = floor(0.5 * (Te_im + Te[i]), 1e-5);
+      // ion temperature
+      const BoutReal tisheath = floor(0.5 * (Ti_im + Ti[i]), 1e-5);


Does it make sense to first interpolate in log space, and then afterwards linear?

What do you mean? Are you talking about interpolating twice?

Yes.

That is what the code does currently.
Ti_im is interpolated in log-space here
and then in linear space, here, 7 lines later.

dschwoerer · 2025-10-21T12:53:50Z

src/sheath_boundary.cxx

+  case SheathLimitMode::exponential_free:
+    fp = SQ(fc) / fm; // Exponential
+  case SheathLimitMode::linear_free:
+    fp = 2.0 * fc - fm; // Linear


Are you not missing the break statement?
Does this not always apply linear_free here?
Maybe a unit test for this would be nice.

Good spot, thanks! I thought this was a warning, but it turns out we don't enable warnings by default

mikekryjak · 2026-01-07T16:24:43Z

src/sheath_boundary.cxx

+};
+
+/// Information about a point on a Y boundary
+struct YBoundary {


Would it make sense to put this somewhere separate so it can be reused in other components?

Yes -- eventually I think this will form the basis of refactoring all the boundary conditions in BOUT++. Once this is in and I start on the next component, I'll see where it's sensible to move it to

Can we please switch instead to YBoundary instead?

This allows to write sheath boundary code for FA and FCI!

More docs is here:
https://bout-dev--3197.org.readthedocs.build/en/3197/user_docs/boundary_options.html#y-boundaries

ZedThree added 22 commits June 25, 2025 17:53

Add basic integrated test for sheath-boundary-simple

c1a0cce

Add unit test for sheath-boundary-simple

3f0fdaf

Fix sheath-boundary-simple when phi isn't set but required

31c5480

`phi` must be calculated in field-aligned coordinates, and transformed to unaligned

Delete some unused code

d8aa3fe

Use enum for sheath boundary condition

655ce43

Clang-tidy fixes to sheath boundaries

a534d0d

Add more cases to sheath boundary simple integrated test

226973a

Add tests for sheath boundary diagnostic quantities

541bbc6

Move sheath phi calculation out into helper function

73a396f

Consolidate loops in calculate_phi

a39e161

Consolidate electron sheath boundary loops

cf92d21

Fix minor bug in calculation of cross-sectional area

53dd502

Lower-y loop used wrong cell face

Pull out repeated calculation of geometric coefficients

71a7001

Make more local variables const

8a3e7a6

Consolidate ion sheath boundary loops

86f9098

Apply clang-format

0515131

Simplify setting sheath diagnostics a little

7b9e28b

Use BoutOutputs instead of collect in test

16e2240

Possibly faster, certainly cleaner

Minor refactoring of sheath test case

0d36c2f

Tests: Fix mirror simple sheath case

6bd97e2

Didn't flip noflow BCs at same time, so was not exactly symmetric

Add integrated test for sheath_boundary

083c0f0

Add helper function and class for looping over Y-boundaries

a890f9f

ZedThree commented Aug 5, 2025

View reviewed changes

src/sheath_boundary_simple.cxx Outdated Show resolved Hide resolved

ZedThree commented Aug 5, 2025

View reviewed changes

ZedThree force-pushed the consolidate-sheath-boundaries branch from e94eb10 to 44e3a0e Compare August 8, 2025 09:01

ZedThree added 2 commits August 8, 2025 14:16

Fix bug from typo in sheath-boundary-insulating

ae5e8a8

Add integrated golden answer test for sheath-boundary-insulating

34099d5

ZedThree added 4 commits August 8, 2025 14:17

Use compile-time switches for different sheath boundary kinds

cfabc91

Merge sheath_boundary_insulating into other sheath_boundarys

b255225

Rename sheath_boundary_simple.{c,h}xx to sheath_boundary.{c,h}xx

b7b0467

ZedThree force-pushed the consolidate-sheath-boundaries branch from 44e3a0e to b7b0467 Compare August 8, 2025 14:08

ZedThree marked this pull request as ready for review August 8, 2025 14:09

ZedThree and others added 3 commits August 18, 2025 17:51

Workaround for CWG2518/P2593R1 for compilers < GCC 13

d9dd7fb

Merge branch 'master' into consolidate-sheath-boundaries

0dcba39

Merge pull request #402 from boutproject/consolidate-sheath-boundarie…

5c673ef

…s-db merge main into consolidate-sheath-boundaries

dschwoerer reviewed Oct 7, 2025

View reviewed changes

dschwoerer reviewed Oct 21, 2025

View reviewed changes

mikekryjak reviewed Jan 7, 2026

View reviewed changes

[WIP] Consolidate sheath boundaries #382

Are you sure you want to change the base?

[WIP] Consolidate sheath boundaries #382

Uh oh!

Conversation

ZedThree commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikekryjak commented Aug 6, 2025

Uh oh!

ZedThree commented Aug 8, 2025

Uh oh!

mikekryjak commented Aug 8, 2025

Uh oh!

ZedThree commented Aug 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ZedThree commented Aug 5, 2025 •

edited

Loading