levelset: surface smoothing

[ksamouchos]

This pull request introduces a new tool which smooths out a discretized surface (polygon or polyhedral). The smoothed surface is continuous and is equipped with a continuous normal. Also, it passes form the discretized geometry's vertices and respects the normal vectors corresponding to each vertex. The new level set computes the projection point and normal based on the new continuous surface.

This pull request completes task IMM-770.

[andrea-iob]

Since it's still a draft, I've only made some random comments without actually testing the changes.

[ksamouchos]

I changed function

void setSurface(const SurfUnstructured *surface, double featureAngle, LevelSetSurfaceSmoothing surfaceSmoothing, bool force = false);

to allow the user to modify the surface smoothing order of object LevelSetSegmentationObject after its creation.

[andrea-iob]

I've still two main concerns:

• I don't see any documentation of how the high order smoothing is evaluated. Did I miss it or it's not there yet?
• I don't understand how the code for updating cell cache could still work if fillCellGeometricNarrowBandLocationCache can return an UNKNOWN location for cells inside the narrow band.

[ksamouchos]

Rebased to the current master

[ksamouchos]

I added a detailed documentation for functions "evalHighOrderProjectionOnLine" and "evalHighOrderProjectionOnTriangle".

[ksamouchos]

I'm answering to the question of @andrea-iob about why I removed the assert from function

bitpit/src/levelset/levelSetSegmentationObject.cpp

Line 2754 in 4040cdd

LevelSetCellLocation LevelSetSegmentationObject::fillCellGeometricNarrowBandLocationCache(long id)

Let's have a look at one of the new integration tests I've added which is not working without removing the assert. The test is the following:

bitpit/test/integration_tests/levelset/test_levelset_00001.cpp

Line 183 in 4040cdd

int subtest_002()

In this case the size of the narrow band is zero. Thus, function "fillCellGeometricNarrowBandLocationCache" finds valoid cell support only for intersected cells. For the rest returns an UNKNOWN location:

bitpit/src/levelset/levelSetSegmentationObject.cpp

Line 2760 in 4040cdd

return LevelSetCellLocation::UNKNOWN;

However, when a smoothed geometry is used, there are cells like the one with id=2824 which are intersected by the discretized geometry but not by the smoothed geometry. These cells have a valid cell support, so the function continues its process by checking if the location is NARROW_BAND_INTERSECTED or NARROW_BAND_DISTANCE in line

bitpit/src/levelset/levelSetSegmentationObject.cpp

Line 2774 in 4040cdd

if (_intersectSurface(id, cellUnsigendValue, CELL_LOCATION_INTERSECTION_MODE) == LevelSetIntersectionStatus::TRUE) {

Cells like the one mentioned above don't belong to any of these two categories meaning that the assert that follows will throw an error. By removing the assert, the code still functions normally, because the location of these cells is declared later in function "fillCartesianCellZoneCache". Either these cells are neighboring the narrow band and detected in line

bitpit/src/levelset/levelSetSegmentationObject.cpp

Line 2713 in 4040cdd

locationCache->insertEntry(neighId, static_cast<char>(LevelSetCellLocation::NARROW_BAND_NEIGHBOUR));

or belong to the bulk and are detected in line

bitpit/src/levelset/levelSetSegmentationObject.cpp

Line 2726 in 4040cdd

locationCache->insertEntry(cellId, static_cast<char>(LevelSetCellLocation::BULK));

More specifically, cell 2824 belongs to the second category.

Consequently, the removal of the assert is necessary and does not harm the code.

[andrea-iob]

Consequently, the removal of the assert is necessary and does not harm the code.

Ok.

Let's avoid caching unknown cell locations (like we already do in LevelSetObject::fillCellGeometricNarrowBandLocationCache) and add a comment explaining that, in the high order case, it's fine to have cells whose cell location will be unknown:

    // Update the cell location cache
    //
    // First we need to check if the cell intersects the surface, and only if it doesn't we
    // should check if its distance is lower than the narrow band size.
    //
    // When high order smoothing is active, there may be cells whose support is within the search
    // radius, but they are not intersected and their distance is less than the narrow band size.
    // These cells are not geometrically inside the narrow band, they are neighbours of cells
    // geometrically inside the narrow band and as such it's up to the caller of this function to
    // identify their cell location.
    LevelSetCellLocation cellLocation = LevelSetCellLocation::UNKNOWN;
    if (_intersectSurface(id, cellUnsigendValue, CELL_LOCATION_INTERSECTION_MODE) == LevelSetIntersectionStatus::TRUE) {
        cellLocation = LevelSetCellLocation::NARROW_BAND_INTERSECTED;
    } else if (cellUnsigendValue <= m_narrowBandSize) {
        cellLocation = LevelSetCellLocation::NARROW_BAND_DISTANCE;
    }
    assert((getSurfaceSmoothing() == LevelSetSurfaceSmoothing::HIGH_ORDER) || (cellLocation != LevelSetCellLocation::UNKNOWN));

    if (cellLocation != LevelSetCellLocation::UNKNOWN) {
        CellCacheCollection::ValueCache<char> *locationCache = getCellCache<char>(m_cellLocationCacheId);
        locationCache->insertEntry(id, static_cast<char>(cellLocation));
    }

[andrea-iob]

If you address the last two comments the pull request can be merged.

[ksamouchos]

Consequently, the removal of the assert is necessary and does not harm the code.

Ok.

Let's avoid caching unknown cell locations (like we already do in LevelSetObject::fillCellGeometricNarrowBandLocationCache) and add a comment explaining that, in the high order case, it's fine to have cells whose cell location will be unknown:

    // Update the cell location cache
    //
    // First we need to check if the cell intersects the surface, and only if it doesn't we
    // should check if its distance is lower than the narrow band size.
    //
    // When high order smoothing is active, there may be cells whose support is within the search
    // radius, but they are not intersected and their distance is less than the narrow band size.
    // These cells are not geometrically inside the narrow band, they are neighbours of cells
    // geometrically inside the narrow band and as such it's up to the caller of this function to
    // identify their cell location.
    LevelSetCellLocation cellLocation = LevelSetCellLocation::UNKNOWN;
    if (_intersectSurface(id, cellUnsigendValue, CELL_LOCATION_INTERSECTION_MODE) == LevelSetIntersectionStatus::TRUE) {
        cellLocation = LevelSetCellLocation::NARROW_BAND_INTERSECTED;
    } else if (cellUnsigendValue <= m_narrowBandSize) {
        cellLocation = LevelSetCellLocation::NARROW_BAND_DISTANCE;
    }
    assert((getSurfaceSmoothing() == LevelSetSurfaceSmoothing::HIGH_ORDER) || (cellLocation != LevelSetCellLocation::UNKNOWN));

    if (cellLocation != LevelSetCellLocation::UNKNOWN) {
        CellCacheCollection::ValueCache<char> *locationCache = getCellCache<char>(m_cellLocationCacheId);
        locationCache->insertEntry(id, static_cast<char>(cellLocation));
    }

Done

[ksamouchos]

Should I squash the commits before merging?

[andrea-iob]

Should I squash the commits before merging?

Keep the commits separate.