Repository navigation
fix: to_global for slices that cover only part of their mesh - #143
Merged
Merged
Conversation
Slice.to_global() placed each subslice using the coordinates of its whole mesh, while the data of a slice defined with XB only covers its own range, so every such slice failed with a reshape ValueError (2D and 3D, node-based and cell-centred, with and without masked=True). Mesh.get_obstruction_mask_slice() likewise returned the mask of the whole mesh plane. A shared helper, extent_index_range(), now gives the nodes or cells a slice covers; SubSlice.get_coordinates(), the obstruction mask and to_global() all use it. The mask now has the shape of the subslice's data. Tested on an FDS 6.11.1 case (slcf_partial_fds611): each partial slice equals the matching section of a full slice in the same plane, with and without the mask. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- to_global takes coord_min/coord_max from the same subslice nodes it places the data with, instead of filtering the mesh coordinates again. - get_obstruction_mask takes an optional region, so the mask of a slice is built only for the cells it covers instead of the whole mesh. - SubSlice.get_coordinates computes the cell centres once. - The test case gets a third mesh with 0.2 m cells; the tests compare against the raw data of every mesh and locate sections by node index instead of the returned coordinates. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The FDS compatibility workflow runs tests/cases/fds_inputs/*.fds through the newest FDS release. The input of the partial XB case is added there, and tests/test_slice_partial_xb.py also runs against that fresh output when FDS_FRESH_OUTPUT_DIR is set, so a change in how FDS writes partial slices shows up when a new release comes out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The test case gets a coarse mesh before the fine ones as well (four meshes: 0.2, 0.1, 0.1, 0.2 m), so both orders are covered. - get_obstruction_mask resolves open ends and negative indices of the region like numpy and rejects steps. - Slice.get_coordinates drops its own nearest-node fallback: every subslice now provides at least one coordinate per axis. - Tests: the region test cuts through each mesh's obstruction and checks that cells are masked, the section comparison checks that partial and full slices give the same number of grids, and the fallback test runs without patching SubSlice.get_coordinates. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Slice.to_global()failed for every slice defined withXBthat does not cover its whole mesh, with and withoutmasked=True:This affected 2D and 3D slices, node-based and cell-centred, across several meshes or inside one, so in practice every 3D slice, which FDS always defines with
XB. For example, with two meshes at x 0..1 and 1..2:FDS writes x 0.3..1.0 into the first mesh and 1.0..1.6 into the second, but
to_global()placed each part as if it covered its whole mesh.Fix
extent_index_range()(fdsreader/utils/extent.py), gives the nodes or cells a slice covers along one axis of its mesh.SubSlice.get_coordinates(), the obstruction mask andto_global()all use it, so they agree on the range.to_global()places each subslice by its own nodes instead of those of its mesh.Mesh.get_obstruction_mask()takes an optionalregionof index ranges and builds the mask only there.Behaviour change:
Mesh.get_obstruction_mask_slice()now returns the mask in the shape of the subslice's data (for 2D slices with the orientation axis of length 1). Before, it covered the whole mesh plane, or the whole mesh for 3D slices, which only differs for slices that cover part of their mesh.Results for slices that cover their meshes are unchanged:
to_global()with and without mask gives bit-identical arrays on all test cases and the example cases.Tests
New fixture
slcf_partial_fds611(FDS 6.11.1, 134 KB): three meshes along x, two at 0.1 m and one at 0.2 m, two obstructions, and in the same planes full slices and slices limited withXB(2D and 3D, node-based and cell-centred, plus one inside a single mesh).tests/test_slice_partial_xb.pychecks that each partial slice equals the matching section of the full slice in the same plane, with and without the mask, that the global grid holds the raw data of every mesh (including the coarser one), that the obstruction is masked, and that the mask has the shape of the subslice's data.The input is also added to
tests/cases/fds_inputs/(slcf_partial.fds), so the FDS compatibility workflow reruns it with each new FDS release and the same tests run against that fresh output.Not part of this PR: which cell plane and which cells the mask marks for cell-centred slices, and the coordinates
to_global(return_coordinates=True)returns for cell-centred slices; both belong to #132.🤖 Generated with Claude Code