[Bug]: SliceCollection.get_nearest returns the first slice listed, not the nearest
Nobody has claimed this yet.
Assessment
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Newbie friendliness
- 76/100
Research direction
Start in fdsreader/slcf/slice_collection.py at SliceCollection.get_nearest and reproduce the ordering-dependent stand-in cases from the issue. Apply the nearest-distance behavior there and make the corresponding change in fdsreader/slcf/geomslice_collection.py. Done means tied nearest slices remain eligible, closer slices replace earlier candidates, and results no longer depend on slice order.
Written by the indexing model from the issue text.
Description
Environment
- fdsreader 1.11.7 (from PyPI).
masterat
61fa6f3
(1.12.1) has the same logic; only the formatting differs. - FDS 6.10.1 (
FDS-6.10.1-0-g12efa16-release) - Python 3.12.13, macOS
Description
SliceCollection.get_nearest(x, y, z) should return the slice nearest to the
point. If several slices of the same quantity are listed in a different order,
it can return a slice that is farther away.
The loop appends every slice whose distance is <= d_min, and it never drops
the slices collected before a closer one turns up:
for slc in self:
...
d = np.sqrt(dx * dx + dy * dy + dz * dz)
if d <= d_min:
d_min = d
slices_min.append(slc)
...
if z is not None:
slices_min.sort(key=lambda slc: slc.extent.z_end - slc.extent.z_start)
The first slice is always appended, because every distance is <= d_min
when d_min starts at finfo(float).max. The list then only grows, and the
sort by extent width that follows does not use distance. Every horizontal
slice has a z-width of 0, and the sort is stable. So when the horizontal
slices cover the same plan area, get_nearest returns the first one listed,
self[0], whatever z is asked for.
GeomSliceCollection.get_nearest has the same pattern
(geomslice_collection.py#L47-L49).
ObstructionCollection.get_nearest uses < and keeps a single result, so it
is not affected.
Minimal reproducible example
1. Without FDS (only fdsreader)
get_nearest reads nothing but slc.extent, so simple stand-in objects are
enough:
from types import SimpleNamespace
from fdsreader.slcf.slice_collection import SliceCollection
def fake_slice(name, x, y, z):
"""Only .extent is read by get_nearest; x, y, z are (start, end) pairs."""
extent = SimpleNamespace(x_start=x[0], x_end=x[1],
y_start=y[0], y_end=y[1],
z_start=z[0], z_end=z[1])
return SimpleNamespace(name=name, extent=extent)
plan = ((-5.0, 5.0), (-5.0, 5.0))
# Case 1: three horizontal slices, listed as 2.0, 1.6, 0.5; query z = 1.6.
slices = SliceCollection([
fake_slice("PBZ=2.0", *plan, (2.0, 2.0)),
fake_slice("PBZ=1.6", *plan, (1.6, 1.6)),
fake_slice("PBZ=0.5", *plan, (0.5, 0.5)),
])
print("case 1: expected PBZ=1.6, got", slices.get_nearest(0.0, 0.0, 1.6).name)
print("case 1 at z=0.5: expected PBZ=0.5, got",
slices.get_nearest(0.0, 0.0, 0.5).name)
# Case 2: the same, but listed as 0.5, 2.0, 1.6.
slices = SliceCollection([
fake_slice("PBZ=0.5", *plan, (0.5, 0.5)),
fake_slice("PBZ=2.0", *plan, (2.0, 2.0)),
fake_slice("PBZ=1.6", *plan, (1.6, 1.6)),
])
print("case 2: expected PBZ=1.6, got", slices.get_nearest(0.0, 0.0, 1.6).name)
# Case 3: a vertical slice through the query point listed first. It contains
# the point (distance 0), so returning it matches the docstring; the case only
# shows that a caller cannot ask for the nearest horizontal slice.
slices = SliceCollection([
fake_slice("PBY=0.0", (-5.0, 5.0), (0.0, 0.0), (0.0, 3.0)),
fake_slice("PBZ=0.5", *plan, (0.5, 0.5)),
fake_slice("PBZ=2.0", *plan, (2.0, 2.0)),
])
print("case 3 (vertical slice contains the point): got",
slices.get_nearest(0.0, 0.0, 1.9).name)
Output with fdsreader 1.11.7:
case 1: expected PBZ=1.6, got PBZ=2.0
case 1 at z=0.5: expected PBZ=0.5, got PBZ=2.0
case 2: expected PBZ=1.6, got PBZ=0.5
case 3 (vertical slice contains the point): got PBY=0.0
Cases 1 and 2 hold the same three slices and differ only in order. Each
returns the first slice listed, whatever the query height.
2. With a real FDS case
This script writes the case, runs FDS (1 s, one 4 x 4 x 6 mesh) and queries
the slices. Set FDS to the FDS executable.
import os
import subprocess
from pathlib import Path
import fdsreader
from fdsreader.slcf.slice_collection import SliceCollection
FDS = os.environ.get("FDS", "fds") # path to the FDS executable
CASE = Path("get_nearest_case")
DECK = """&HEAD CHID='get_nearest_case' /
&MESH IJK=4,4,6, XB=0.0,4.0,0.0,4.0,0.0,3.0 /
&TIME T_END=1.0 /
&SLCF PBY=2.0, QUANTITY='TEMPERATURE' /
&SLCF PBZ=2.0, QUANTITY='TEMPERATURE' /
&SLCF PBZ=1.6, QUANTITY='TEMPERATURE' /
&SLCF PBZ=0.5, QUANTITY='TEMPERATURE' /
&TAIL /
"""
CASE.mkdir(exist_ok=True)
(CASE / "get_nearest_case.fds").write_text(DECK)
subprocess.run([FDS, "get_nearest_case.fds"], cwd=CASE, check=True,
stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
sim = fdsreader.Simulation(str(CASE))
slices = sim.slices.filter_by_quantity("TEMPERATURE")
for i, slc in enumerate(slices):
print(i, slc.orientation, slc.extent)
# Point (2.0, 2.0, 1.6): the PBZ=1.6 slice (written at z = 1.5 on this
# 0.5 m grid) is 0.1 m away, PBZ=2.0 is 0.4 m away.
# The vertical PBY=2.0 slice contains the point, so it wins over all slices
# (by design); the horizontal-only collection shows the bug.
print("all slices :", slices.get_nearest(2.0, 2.0, 1.6).extent)
horizontal = SliceCollection([s for s in slices if s.orientation == 3])
print("horizontal only:", horizontal.get_nearest(2.0, 2.0, 1.6).extent)
Output with fdsreader 1.11.7 and FDS 6.10.1 (warnings removed):
0 2 Extent([0.00, 4.00] x [2.00, 2.00] x [0.00, 3.00])
1 3 Extent([0.00, 4.00] x [0.00, 4.00] x [2.00, 2.00])
2 3 Extent([0.00, 4.00] x [0.00, 4.00] x [1.50, 1.50])
3 3 Extent([0.00, 4.00] x [0.00, 4.00] x [0.50, 0.50])
all slices : Extent([0.00, 4.00] x [2.00, 2.00] x [0.00, 3.00])
horizontal only: Extent([0.00, 4.00] x [0.00, 4.00] x [2.00, 2.00])
Expected behaviour
get_nearest(2.0, 2.0, 1.6)over the horizontal slices returns the slice at
z = 1.5 (0.1 m away).- The stand-in cases 1 and 2 return
PBZ=1.6, and case 1 at z = 0.5 returns
PBZ=0.5. - The result does not depend on the order of the
&SLCFlines. - The docstring says a random slice is picked only if several slices are at
the same distance.
Actual behaviour
- The first horizontal slice listed, PBZ=2.0 (0.4 m away), is returned.
- In the stand-in example, the first slice listed is returned for every
query height. - With all slices, the vertical PBY=2.0 slice is returned. This is correct by
the docstring, because the point lies inside the vertical slice (distance
0). But a caller who wants the horizontal slice at a height has no way to
ask for one (see the optional part below).
Suggested fix
Reset the candidate list when a strictly closer slice is found, and append
only on ties:
--- a/fdsreader/slcf/slice_collection.py
+++ b/fdsreader/slcf/slice_collection.py
@@ -45,9 +45,11 @@ class SliceCollection(FDSDataCollection):
dy = max(slc.extent.y_start - y, 0, y - slc.extent.y_end) if y is not None else 0
dz = max(slc.extent.z_start - z, 0, z - slc.extent.z_end) if z is not None else 0
d = np.sqrt(dx * dx + dy * dy + dz * dz)
- if d <= d_min:
+ if d < d_min:
d_min = d
- slices_min.append(slc)
+ slices_min = [slc]
+ elif d == d_min:
+ slices_min.append(slc)
if x is not None:
slices_min.sort(key=lambda slc: slc.extent.x_end - slc.extent.x_start)
The same change applies to GeomSliceCollection.get_nearest.
Optional: an orientation argument (None by default) that skips slices of
another orientation, so get_nearest(x, y, z, orientation=3) returns the
nearest horizontal slice even when a vertical slice passes through the point:
- def get_nearest(self, x: float = None, y: float = None, z: float = None) -> Slice:
+ def get_nearest(self, x: float = None, y: float = None, z: float = None,
+ orientation: int = None) -> Slice:
@@
for slc in self:
+ if orientation is not None and slc.orientation != orientation:
+ continue
dx = ...
With both changes applied to the stand-in example (and an orientation
field added to fake_slice), cases 1 and 2 return PBZ=1.6, case 1 at
z = 0.5 returns PBZ=0.5, case 3 still returns PBY=0.0, and case 3 with
orientation=3 returns PBZ=2.0.
Backward compatibility
- The signature is unchanged. The optional argument defaults to the current
behaviour. - Results change only where the current code returns a slice that is not
the nearest. - Code that depended on getting the first slice listed will get the nearest
one instead. That matches the docstring.
Context
We found this through fdsvismap.
Its VisMap.read_fds_data picks the extinction slice with
get_nearest(0, 0, height), so the visibility it computes depended on the
order of the &SLCF lines in the case.
- Dominant language
- Python
- Stars
- 75
- Forks
- 28
- Avg merge
- 2m
- Merged PRs (30d)
- 2
Getting set up
- No Dockerfile or Docker Compose file
- No pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from FireDynamics/fdsreader
-
Difficulty 1/5 Under an hour Newbie friendliness 90/100
FireDynamics/fdsreader#125 ·
-
Slice.to_global puts the second-to-last node row on the domain's upper facesPossibly taken @chraibi claimed this 1 day ago. Open
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
FireDynamics/fdsreader#123 ·
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
FireDynamics/fdsreader#122 · 2 comments ·
-
Difficulty 3/5 1-2 days Newbie friendliness 70/100
FireDynamics/fdsreader#121 ·
-
bug
Difficulty 3/5 1-2 days Newbie friendliness 45/100
FireDynamics/fdsreader#115 · 1 comment ·
All issues in FireDynamics/fdsreader
Similar issues
-
area/install reliability
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 83/100
FluidNumerics/fluid-walk-blocker#191 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 62/100
TransformerLensOrg/TransformerLens#1868 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
Maintainers usually reply within 1 day
-
Difficulty 1/5 Under an hour Newbie friendliness 85/100
climate-analytics-lab/jax-gcm#1057 ·
Maintainers usually reply within 1 day