Ensure no overlap with crinkle clip

[banesullivan]

Overview

Resolves #5667 to ensure that when crinkle clipping and returning the clipped out mesh, the clipped out mesh does not have any of the same cells as the primary.

Details

import pyvista as pv
from pyvista import examples

dataset = examples.download_bunny_coarse()
clipped, unclipped = dataset.clip('y', return_clipped=True, crinkle=True)

pl = pv.Plotter()
pl.add_mesh(clipped, color='blue')
pl.add_mesh(unclipped, color='red')
pl.show()

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.96%. Comparing base (3e9a19d) to head (dfd53f2).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6060   +/-   ##
=======================================
  Coverage   96.96%   96.96%           
=======================================
  Files         141      141           
  Lines       24530    24532    +2     
=======================================
+ Hits        23786    23788    +2     
  Misses        744      744           

[MatthewFlamm]

I am +1 for including this feature, and it makes sense based on the description of the parameters. But, are there also any use cases for returning the crinkle cut from both directions here (current behavior)? I can't think of any obvious ones, but it seems probable that someone would want this behavior.

[banesullivan]

are there also any use cases for returning the crinkle cut from both directions here (current behavior)? I can't think of any obvious ones, but it seems probable that someone would want this behavior.

I also cannot think of any and I think that when calling the clip function with crinkle enabled and wanting both sides of the clip, there should not be any overlap.

If someone wants to crinkle both sides on the overlap, then thats still possible, its just two seperate clips and can be done two ways:

1. By flipping the normal:

a = dataset.clip('y', crinkle=True)
b = dataset.clip('-y', crinkle=True)

2. Using the invert option:

a = dataset.clip('y', crinkle=True, invert=True)
b = dataset.clip('y', crinkle=True, invert=False)

import pyvista as pv
from pyvista import examples

dataset = examples.download_bunny_coarse()
clipped, unclipped = dataset.clip('y', return_clipped=True, crinkle=True)

a = dataset.clip('y', crinkle=True)
b = dataset.clip('-y', crinkle=True)
# a = dataset.clip('y', crinkle=True, invert=True)
# b = dataset.clip('y', crinkle=True, invert=False)

assert (a == clipped)
assert not (b == unclipped)

pl = pv.Plotter(shape=(1,2))
pl.add_mesh(clipped, color='blue')
pl.add_mesh(unclipped, color='red', style='wireframe', line_width=5)
pl.subplot(0, 1)
pl.add_mesh(a, color='blue')
pl.add_mesh(b, color='red', style='wireframe', line_width=5)

pl.link_views()
pl.show()

[MatthewFlamm]

I think it is fine as the PR has it. There could be an unexpected breaking change for someone using the current behavior. We might argue it was unintended behavior, so they should not have been using it? I'm mostly trying to think through whether a 'breaking change' label should be applied, or this is just a bug fix. Not trying to change the PR itself.

I thought of one case, two crinkle cuts that are offset in space are used in opposing directions to obtain a sliver of cells to compare cell size distribution (common in CFD for example). Currently, a user could achieve this by either inverting one clip or using return_clip=True from one clip.

[MatthewFlamm]

Small suggestion to consider. Also consider labeling breaking change. Im only 50/50 on both.

[banesullivan]

While in my opinion this is a bug and unintended behavior I think you provide a great example and I want to note that this bug has been around for 2 years (ref #2316). I think a breaking change label to denote that the output mesh is indeed different is appropriate here.