Exclude faces that touch NaN nodes from the SpatialHash table - #2802
Conversation
…/lon grid before injecting nan values. Otherwise the nan values break other tests.
|
@erikvansebille, curious to hear your thoughts on this solution! To me it seemed like this approach would take the least amount of refactoring as it doesn't mess with the 2d array reconfiguration issues you had mentioned earlier today. |
|
This works beautifully! My example that I showed earlier in #2796 (review) now looks much better |
| return int((nx * ny * nz).sum()) | ||
| # NaN values are not allowed in the SpatialHash table, so faces with a NaN | ||
| # bounding box do not contribute to the entry count | ||
| invalid_face = ( |
There was a problem hiding this comment.
We tend to think of masks here, so perhaps rename to valid_mask?
There was a problem hiding this comment.
Agreed, I modified the logic so that the True corresponds to valid non-NaN cells.
…ode/Parcels into spatialhash-NaN-cell-treatment
…e is less than the total number of mesh faces when NaNs are in the mesh.
for more information, see https://pre-commit.ci
erikvansebille
left a comment
erikvansebille
left a comment
There was a problem hiding this comment.
Nice! One more small suggestion
Co-authored-by: Erik van Sebille <e.vansebille@uu.nl>
erikvansebille
left a comment
erikvansebille
left a comment
There was a problem hiding this comment.
Very good! Now it's custom that you, as the initiator of this PR, also merge it. DO you have the rights to do that?
|
Just initiated it, I believe it should go through when the checks complete? |
2 participants
Description
Previously, any faces that touched NaN nodes in curvilinear or unstructured grids were treated as a normal face. This meant they were quantized and added to the SpatialHash table. The resulting behavior was a pileup of faces being assigned to (0, 0, 0). For more details see the discussion on #2796. This PR resolves this issue by manually setting values in
num_hash_per_faceto 0 for faces which touch a node that is NaN. This prevents these faces from ever entering the SpatialHash table. This PR also adds testing to confirm that faces that touch NaN nodes are never in the SpatialHash table, and that this new behavior does not effect the query resolution of any other mesh cells in the grid.Checklist
mainfor normal development,v3-supportfor v3 support)AI Disclosure