Hacktoberfest 2026: the issues maintainers tagged for October, open and beginner-friendly. Browse Hacktoberfest issues

[Bug]: SliceCollection.get_nearest returns the first slice listed, not the nearest

Open
#120 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
76/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
numpy, python
Domain
data

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). master at
    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:

https://github.com/FireDynamics/fdsreader/blob/61fa6f321888f354b3b14d5a3449f9be94d17f1e/fdsreader/slcf/slice_collection.py#L43-L57

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 &SLCF lines.
  • 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

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from FireDynamics/fdsreader

All issues in FireDynamics/fdsreader

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.