Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

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

Aperta
#120 2 commenti 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
3/5
Tempo stimato
1-2 giorni
Idoneità per principianti
76/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
numpy, python
Ambito
data

Direzione di ricerca

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.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

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.

Lingua principale
Python
Stelle
75
Fork
28
Merge medio
2m
PR unite (30g)
2

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di FireDynamics/fdsreader

Tutte le issue di FireDynamics/fdsreader

Issue simili

Altre issue su Python

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.