Exclude faces that touch NaN nodes from the SpatialHash table - #2802
Exclude faces that touch NaN nodes from the SpatialHash table#2802wyatt-fluidnumerics wants to merge 7 commits into
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 |
There was a problem hiding this comment.
This solution is very elegant! Nice work @wyatt-fluidnumerics
The only thing I wonder about is if it's worth to make a helper function to construct the invalid_face array. Because the same code is now used twice. Or will that make going through the code only more confusing?
| 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?
| "Total hash cells": f"{n_total_cells:,d}", | ||
| "Occupied hash cells": f"{n_occupied_cells:,d}, {n_occupied_cells / n_total_cells * 100:.4f}%", | ||
| "Total (hash cell --> gird face) entries": f"{n_entries:,d}", | ||
| "Total (hash cell --> grid face) entries": f"{n_entries:,d}", |
There was a problem hiding this comment.
Left-over typo from #2796 that I only spotted now...
There was a problem hiding this comment.
Nice test, but should we also add a check that the number of unique mesh faces is less than the number of original grid cells - because the NaNs are filtered out?
I think this should be len(np.unique(spatialhash._hash_table["faces"])), but double-check. Perhaps that's also what the "Total mesh faces" of hashtable.describe() should show?
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