[Bug]: SliceCollection.get_nearest returns the first slice listed, not the nearest
Maintainer thường phản hồi trong vòng 1 ngày
Chưa có ai nhận issue này.
Đánh giá
- Độ khó
- 3/5
- Thời gian dự kiến
- 1-2 ngày
- Mức phù hợp với người mới
- 76/100
Hướng nghiên cứu
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.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Mô tả
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.
- Ngôn ngữ chính
- Python
- Star
- 76
- Fork
- 28
- Merge trung bình
- 16 giờ 49 phút
- Pull request đã merge (30 ngày)
- 28
Chuẩn bị môi trường
- Không có Dockerfile hay tệp Docker Compose
- Không có mẫu pull request
- Đọc hướng dẫn đóng góp
Bắt đầu từ đâu
- Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
- Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
- Fork repository và làm thay đổi trên một nhánh.
- Mở pull request có tham chiếu số hiệu của issue.
Issue khác của FireDynamics/fdsreader
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 70/100
FireDynamics/fdsreader#133 · 1 bình luận ·
Maintainer thường phản hồi trong vòng 1 ngày
-
Get RGBA-Value of obstruction from Surf_IdCó thể làm lại được @goelakash đã nhận 40 ngày trước và không có pull request nào đang mở. Đang mởenhancement
Độ khó 4/5 3-5 ngày Mức phù hợp với người mới 35/100
FireDynamics/fdsreader#77 · 3 bình luận ·
Maintainer thường phản hồi trong vòng 1 ngày
-
to_global has Problem with different cell-sizesCó thể làm lại được @lu-kas đã nhận 837 ngày trước và không có pull request nào đang mở. Đang mở
FireDynamics/fdsreader#49 · 8 bình luận · 1 người được giao ·
Maintainer thường phản hồi trong vòng 1 ngày
-
[Feature]: Parallel readingĐang mởenhancement
Độ khó 5/5 Hơn một tuần Mức phù hợp với người mới 25/100
FireDynamics/fdsreader#23 · 2 bình luận ·
Maintainer thường phản hồi trong vòng 1 ngày
Tất cả issue của FireDynamics/fdsreader
Issue tương tự
-
Use mhctools predictor_required=False for saved-result rescoringCó thể đã có người làm @iskandr đã nhận hôm nay. Đang mở
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 72/100
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 79/100
-
Độ khó 1/5 Dưới một giờ Mức phù hợp với người mới 85/100
521xueweihan/HelloGitHub#4051 · 1 bình luận ·
-
bug documentation
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 78/100
awslabs/visual-asset-management-system#514 ·
Maintainer thường phản hồi trong vòng 1 ngày
-
backend bug spec:REQ-053
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 66/100
nolte/kamerplanter#2547 ·
Maintainer thường phản hồi trong vòng 1 ngày