[Bug]: SliceCollection.get_nearest returns the first slice listed, not the nearest
Nessuno ha ancora preso questa issue.
Valutazione
- Difficoltà
- 3/5
- Tempo stimato
- 1-2 giorni
- Idoneità per principianti
- 76/100
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).
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.
- Lingua principale
- Python
- Stelle
- 75
- Fork
- 28
- Merge medio
- 2m
- PR unite (30g)
- 2
Preparare l'ambiente
- Nessun Dockerfile né file Docker Compose
- Nessun modello di pull request
- Leggi la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di FireDynamics/fdsreader
-
bug
Difficoltà 3/5 1-2 giorni Idoneità per principianti 45/100
FireDynamics/fdsreader#115 ·
-
Difficoltà 4/5 3-5 giorni Idoneità per principianti 35/100
FireDynamics/fdsreader#99 · 11 commenti ·
-
bug
Difficoltà 3/5 1-2 giorni Idoneità per principianti 72/100
FireDynamics/fdsreader#89 · 2 commenti ·
-
Difficoltà 4/5 3-5 giorni Idoneità per principianti 25/100
FireDynamics/fdsreader#81 ·
-
enhancement help wanted
Difficoltà 4/5 3-5 giorni Idoneità per principianti 35/100
FireDynamics/fdsreader#77 · 3 commenti ·
Tutte le issue di FireDynamics/fdsreader
Issue simili
-
Difficoltà 1/5 Meno di un'ora Idoneità per principianti 72/100
letsencrypt/cp-cps#353 ·
-
Marble Madness II is missingAperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 68/100
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 84/100
PedestrianDynamics/pyFDS-Evac#394 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
DOI-USGS/pywatershed#421 ·
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
python-pillow/Pillow#10087 · 1 commento ·
I maintainer di solito rispondono entro 1 giorno