Skip to content

Add information about which cells have sheath boundaries - #3468

Closed
totork wants to merge 5 commits into
nextfrom
coords-hasbndry
Closed

Add information about which cells have sheath boundaries#3468
totork wants to merge 5 commits into
nextfrom
coords-hasbndry

Conversation

@totork

@totork totork commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Currently, there is no straight forward way in Fci to determine if a cell has adjacent parallel sheath boundaries inside operators. This prevents e.g. turning of parallel diffusion into the boundaries, as explained in

#3462

The goal of this PR is to add this information as a member of coordinates. This is then easily accessible inside operators and can be used to adjust the calculation if needed. This is also a first try, I am not sure if there is an easier to way do this.

@totork
totork requested a review from dschwoerer August 21, 2026 10:47
@totork
totork marked this pull request as draft August 21, 2026 10:47

@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

}

const FieldMetric& cell_sheath_ylow() const {
if (_cell_sheath_ylow.has_value()) {

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.

warning: use of undeclared identifier '_cell_sheath_ylow' [clang-diagnostic-error]

    if (_cell_sheath_ylow.has_value()) {
        ^


const FieldMetric& cell_sheath_ylow() const {
if (_cell_sheath_ylow.has_value()) {
return *_cell_sheath_ylow;

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.

warning: reference to non-static member function must be called [clang-diagnostic-error]

      return *_cell_sheath_ylow;
              ^


const FieldMetric& cell_sheath_ylow() const {
if (_cell_sheath_ylow.has_value()) {
return *_cell_sheath_ylow;

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.

warning: use of undeclared identifier '_cell_sheath_ylow'; did you mean 'cell_sheath_ylow'? [clang-diagnostic-error]

Suggested change
return *_cell_sheath_ylow;
return *cell_sheath_ylow;
Additional context

include/bout/coordinates.hxx:302: 'cell_sheath_ylow' declared here

  const FieldMetric& cell_sheath_ylow() {
                     ^

return *_cell_sheath_ylow;
}
_determine_cell_sheath();
ASSERT2(_cell_sheath_ylow.has_value());

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.

warning: use of undeclared identifier '_cell_sheath_ylow' [clang-diagnostic-error]

    ASSERT2(_cell_sheath_ylow.has_value());
            ^

}
_determine_cell_sheath();
ASSERT2(_cell_sheath_ylow.has_value());
return *_cell_sheath_ylow;

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.

warning: reference to non-static member function must be called [clang-diagnostic-error]

    return *_cell_sheath_ylow;
            ^

Comment thread src/mesh/coordinates.cxx
_cell_area_zhigh.reset();
_cell_volume.reset();
_cell_sheath_yhigh.reset();
_cell_sheath_ylow.reset();

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.

warning: use of undeclared identifier '_cell_sheath_ylow'; did you mean 'cell_sheath_ylow'? [clang-diagnostic-error]

Suggested change
_cell_sheath_ylow.reset();
cell_sheath_ylow.reset();
Additional context

include/bout/coordinates.hxx:293: 'cell_sheath_ylow' declared here

  const FieldMetric& cell_sheath_ylow() const {
                     ^

Comment thread src/mesh/coordinates.cxx
BOUT_OMP_SAFE(critical)
{
if (!_cell_volume.has_value()) {
FieldMetric sheath_yhigh = 0.0;

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.

warning: variable 'sheath_yhigh' of type 'FieldMetric' (aka 'Field2D') can be declared 'const' [misc-const-correctness]

Suggested change
FieldMetric sheath_yhigh = 0.0;
FieldMetric const sheath_yhigh = 0.0;

Comment thread src/mesh/coordinates.cxx
{
if (!_cell_volume.has_value()) {
FieldMetric sheath_yhigh = 0.0;
FieldMetric sheath_ylow = 0.0;

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.

warning: variable 'sheath_ylow' of type 'FieldMetric' (aka 'Field2D') can be declared 'const' [misc-const-correctness]

Suggested change
FieldMetric sheath_ylow = 0.0;
FieldMetric const sheath_ylow = 0.0;

Comment thread src/mesh/coordinates.cxx
if (!_cell_volume.has_value()) {
FieldMetric sheath_yhigh = 0.0;
FieldMetric sheath_ylow = 0.0;
YBoundary sheathbndry(YBndryType::sheath, nullptr, *localmesh);

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.

warning: variable 'sheathbndry' of type 'YBoundary' can be declared 'const' [misc-const-correctness]

Suggested change
YBoundary sheathbndry(YBndryType::sheath, nullptr, *localmesh);
YBoundary const sheathbndry(YBndryType::sheath, nullptr, *localmesh);

Comment thread src/mesh/coordinates.cxx Outdated
if (pnt.dir() > 0) {
sheath_yhigh[i] = 1;
} else {
heath_ylow[i] = 1;

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.

warning: use of undeclared identifier 'heath_ylow'; did you mean 'sheath_ylow'? [clang-diagnostic-error]

Suggested change
heath_ylow[i] = 1;
sheath_ylow[i] = 1;
Additional context

src/mesh/coordinates.cxx:1230: 'sheath_ylow' declared here

      FieldMetric sheath_ylow = 0.0;
                  ^

@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/coordinates.cxx
void Coordinates::_determine_cell_sheath() const {
BOUT_OMP_SAFE(critical)
{
if (!_cell_sheath_yhigh.has_value() || !_cell_sheath_ylow.has_value()) {

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.

warning: use of undeclared identifier '_cell_sheath_ylow' [clang-diagnostic-error]

    if (!_cell_sheath_yhigh.has_value() || !_cell_sheath_ylow.has_value()) {
                                            ^

@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/coordinates.cxx
}
}
}
_cell_sheath_yhigh.emplace(sheath_yhigh);

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.

warning: expected ')' [clang-diagnostic-error]

      _cell_sheath_yhigh.emplace(sheath_yhigh);
      ^
Additional context

src/mesh/coordinates.cxx:1233: to match this '('

      sheathbndry.iter([&](auto& pnt) {
                      ^

Comment thread src/mesh/coordinates.cxx
Comment on lines +1226 to +1248
void Coordinates::_determine_cell_sheath() const {
BOUT_OMP_SAFE(critical)
{
if (!_cell_sheath_yhigh.has_value() || !_cell_sheath_ylow.has_value()) {
FieldMetric sheath_yhigh = 0.0;
FieldMetric sheath_ylow = 0.0;
YBoundary sheathbndry(YBndryType::sheath, nullptr, *localmesh);

sheathbndry.iter([&](auto& pnt) {
const auto& i = pnt.ind();
if (abs(pnt.offset()) == 1) {
if (pnt.dir() > 0) {
sheath_yhigh[i] = 1;
} else {
sheath_ylow[i] = 1;
}
}
});
_cell_sheath_yhigh.emplace(sheath_yhigh);
_cell_sheath_ylow.emplace(sheath_ylow);
}
}
}

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.

I do not think we should duplicate the code from YBoundary.

Also, this prevents the user from configuring which y boundary actually is a sheath boundary.

I think it would be nicer if we would store a YBoundary object in coordinates, set it up properly, i.e. with an Options object passed in, and then have an API to get a shared_ptr to that YBoundary object.

Then we also do not need to duplicate the YBoundary API.

Also, I doubt we need to clear the cache in any case, I would not be aware that we allow to change any of the things in a mesh, which would make a recomputation of it necessary.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I see, makes sense. So I can close this PR? Would you implement this? I am not sure what you mean with your suggestion.

@dschwoerer

Copy link
Copy Markdown
Contributor

Ah, it is already there, just the docs need fixing: #3469

@dschwoerer dschwoerer closed this Aug 21, 2026
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.

2 participants