Clip: Fix cellOffset increment #104
Conversation
In GenerateCellSet, if the cell was kept, we forgot to increment the cellOffset, which lead to writing the types and offsets at the wrong memory location. We also made sure that all offset increments are correct and documented.
ScanExclusive can only be executed only on filled batches instead of all batches, using the output to input map from mask select
|
The ScanExclusive changes are currently tested on Frontier to check if they are useful or not. |
|
Old Data is plain ScanExclusive , New Data is ScanExclusive using MaskSelect's Output to Input array: F-15: 246,056,538 cells, 116,414,075 points
JSM-tet: 299,608,989 cells, 50,429,916 points
Based on the figures i see here: The highest time remained pretty much the same, the average dropped, and the minimum is ROCK bottom as expected. I feel like we should integrate these changes. What do you think? |
|
@kmorel the indexing failure was not caught because the UnitTestClipWithFieldFilter.cxx/UnitTestClipWithImplicitFunctionFilter.cxx check only for the number of cells/points/fields, and some scalar values. They don't check cell types or offsets. |
That makes sense. I was just surprised this didn't show up in any renderings. The Ascent folks used the clip in their renderings and didn't see missing or malformed cells. |
|
@spyridon97 Does GitHub give you permissions to merge this PR, or do I need to do it? |
I get "You're not authorized to push to this branch. Visit https://docs.github.com/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches for more information." |
That's fine. I merged this. |
|
I think i was given Triage rights when you invited me to the project. |
That is correct. With the added formality of the charter document with the LF, I'm trying to limit the number of people with write access. I just wasn't sure if GitHub allowed you to merge a PR once all the requirements are met. I guess you need permission to write to protected branches for that. |
There was bug in the newly redesigned Clip filter that caused the cellOffset to not be incremented,
cause output cells to be incorrectly placed in the output. This has now been fixed, and the output.
Additionally, all resources are released as soon as possible.
Finally, the ScanExclusive usages within the Clip filter have been improved to operate
only on the batches of points/cells that are actually needed, rather than the entire batch data.