fix(understand): the pipe network's tolerance follows the P&ID's unit, and a dropped stub is said
pipe_takeoff merged line ends closer than tol = 1.0 drawing units and silently dropped any segment shorter than that. On a millimetre P&ID that is 1 mm. On a metre drawing it is a metre: ends a metre apart were merged and every pipe under a metre disappeared from the totals, with no warning. The review of the network engine found this. - tol now defaults to 1 mm in the unit the P&ID's geometry implies: 1.0 on a millimetre drawing, 0.001 on a metre one. A tol the caller gives is used as given. - build_network counts the segments that collapse to a point (stats.segments_below_tol), and pipe_rows turns a non-zero count into a warning naming the tolerance. A new wire test builds a metre P&ID with a 0.6 m pipe and a 5 m pipe. Before this change the takeoff said 5.0 m; now it says 5.6 m. The network test checks that a sub-tolerance stub is counted. Registry, packs, aliases and every track H suite: 472 passed. The inventory matches the live surface. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
fb2953c324
commit
c4759e696d
@@ -21,6 +21,7 @@ from engineering.understand.takeoff import (
|
|||||||
check_pipe_options,
|
check_pipe_options,
|
||||||
check_section_rules,
|
check_section_rules,
|
||||||
pipe_rows,
|
pipe_rows,
|
||||||
|
unit_of,
|
||||||
)
|
)
|
||||||
|
|
||||||
__all__ = ["run_cable_takeoff", "run_pipe_takeoff"]
|
__all__ = ["run_cable_takeoff", "run_pipe_takeoff"]
|
||||||
@@ -49,7 +50,7 @@ async def run_pipe_takeoff(
|
|||||||
length_source: str = "auto",
|
length_source: str = "auto",
|
||||||
force: bool = False,
|
force: bool = False,
|
||||||
layers=None,
|
layers=None,
|
||||||
tol: float = 1.0,
|
tol: float | None = None,
|
||||||
) -> dict:
|
) -> dict:
|
||||||
"""`pipe_rows` over a P&ID and an optional layout, plus the workbook when `output` is named.
|
"""`pipe_rows` over a P&ID and an optional layout, plus the workbook when `output` is named.
|
||||||
|
|
||||||
@@ -64,14 +65,17 @@ async def run_pipe_takeoff(
|
|||||||
vertical_allowance=vertical_allowance,
|
vertical_allowance=vertical_allowance,
|
||||||
length_source=length_source,
|
length_source=length_source,
|
||||||
)
|
)
|
||||||
tol = _check_tol(tol)
|
tol = None if tol is None else _check_tol(tol)
|
||||||
if length_source == "layout" and not layout_path:
|
if length_source == "layout" and not layout_path:
|
||||||
raise ValueError("length_source='layout' needs a layout drawing; none was given")
|
raise ValueError("length_source='layout' needs a layout drawing; none was given")
|
||||||
pid = await take_snapshot(backend, pid_path)
|
pid = await take_snapshot(backend, pid_path)
|
||||||
layout = await take_snapshot(backend, layout_path) if layout_path else None
|
layout = await take_snapshot(backend, layout_path) if layout_path else None
|
||||||
|
|
||||||
def compute() -> dict:
|
def compute() -> dict:
|
||||||
network = build_network(pid, layers=layers, tol=tol)
|
# 1 mm in the unit the P&ID's geometry implies: a fixed 1.0 would be a
|
||||||
|
# metre on a metre drawing, merging ends a metre apart and dropping pipes
|
||||||
|
joint = tol if tol is not None else 1.0 / (unit_of(pid)["m_per_unit"] * 1000.0)
|
||||||
|
network = build_network(pid, layers=layers, tol=joint)
|
||||||
return pipe_rows(
|
return pipe_rows(
|
||||||
network,
|
network,
|
||||||
pid,
|
pid,
|
||||||
|
|||||||
@@ -539,10 +539,13 @@ def build_network(snap: Snapshot, *, layers=None, tol=1.0, label_search=None) ->
|
|||||||
raw = _segments(snap, chosen)
|
raw = _segments(snap, chosen)
|
||||||
verts = _Vertices(tol)
|
verts = _Vertices(tol)
|
||||||
snapped = []
|
snapped = []
|
||||||
|
below_tol = 0 # segments whose two ends merge into one vertex: dropped, and counted
|
||||||
for a, b, bulge, handle, layer in raw:
|
for a, b, bulge, handle, layer in raw:
|
||||||
u, v = verts.index(a), verts.index(b)
|
u, v = verts.index(a), verts.index(b)
|
||||||
if u != v:
|
if u != v:
|
||||||
snapped.append((u, v, bulge, handle, layer))
|
snapped.append((u, v, bulge, handle, layer))
|
||||||
|
else:
|
||||||
|
below_tol += 1
|
||||||
|
|
||||||
# equipment first: an INSERT a tag labels is never a fitting
|
# equipment first: an INSERT a tag labels is never a fitting
|
||||||
occurrences = tag_occurrences(snap)
|
occurrences = tag_occurrences(snap)
|
||||||
@@ -888,6 +891,7 @@ def build_network(snap: Snapshot, *, layers=None, tol=1.0, label_search=None) ->
|
|||||||
"tol": tol,
|
"tol": tol,
|
||||||
"label_search": search,
|
"label_search": search,
|
||||||
"segments": len(raw),
|
"segments": len(raw),
|
||||||
|
"segments_below_tol": below_tol,
|
||||||
"pieces": len(pieces),
|
"pieces": len(pieces),
|
||||||
"overlap_removed": overlap_count,
|
"overlap_removed": overlap_count,
|
||||||
"overlap_length": overlap_length,
|
"overlap_length": overlap_length,
|
||||||
|
|||||||
@@ -802,6 +802,13 @@ def pipe_rows(
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
warnings = [u["warning"] for u in (pid_unit, layout_unit) if u and u["warning"]]
|
warnings = [u["warning"] for u in (pid_unit, layout_unit) if u and u["warning"]]
|
||||||
|
dropped = int(network.get("stats", {}).get("segments_below_tol") or 0)
|
||||||
|
if dropped:
|
||||||
|
warnings.append(
|
||||||
|
f"{dropped} P&ID segment(s) shorter than the junction tolerance "
|
||||||
|
f"({network['stats']['tol']:g} drawing units) collapsed to a point and are not "
|
||||||
|
"counted"
|
||||||
|
)
|
||||||
if not scale_verified:
|
if not scale_verified:
|
||||||
warnings.append(
|
warnings.append(
|
||||||
"scale_verified: false - lengths are drawn lengths on a P&ID that no scale check "
|
"scale_verified: false - lengths are drawn lengths on a P&ID that no scale check "
|
||||||
|
|||||||
@@ -11124,13 +11124,15 @@ async def pipe_takeoff(
|
|||||||
),
|
),
|
||||||
] = None,
|
] = None,
|
||||||
tol: Annotated[
|
tol: Annotated[
|
||||||
float,
|
float | None,
|
||||||
Field(
|
Field(
|
||||||
default=1.0,
|
default=None,
|
||||||
gt=0.0,
|
gt=0.0,
|
||||||
description="Line ends closer than this (P&ID drawing units) are one junction.",
|
description="Line ends closer than this (P&ID drawing units) are one junction; "
|
||||||
|
"default 1 mm in the unit the P&ID's geometry implies (1.0 on a millimetre "
|
||||||
|
"drawing, 0.001 on a metre one).",
|
||||||
),
|
),
|
||||||
] = 1.0,
|
] = None,
|
||||||
ctx: Context = None,
|
ctx: Context = None,
|
||||||
) -> dict:
|
) -> dict:
|
||||||
"""A pipe takeoff: topology from the P&ID, lengths from the layout, rows by room x service x diameter.
|
"""A pipe takeoff: topology from the P&ID, lengths from the layout, rows by room x service x diameter.
|
||||||
|
|||||||
@@ -103,6 +103,14 @@ def test_ends_within_tol_merge_into_one_run():
|
|||||||
assert run.ends == ((0.0, 0.0), (2000.0, 0.0))
|
assert run.ends == ((0.0, 0.0), (2000.0, 0.0))
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_segment_shorter_than_tol_is_counted_when_it_is_dropped():
|
||||||
|
# a 0.5-unit stub collapses to one vertex at tol 1.0: gone from the network,
|
||||||
|
# but never silently - stats say how many went
|
||||||
|
result = net(line("1", (0.0, 0.0), (1000.0, 0.0)), line("2", (2000.0, 0.0), (2000.5, 0.0)))
|
||||||
|
assert result["stats"]["segments_below_tol"] == 1
|
||||||
|
assert net(line("1", (0.0, 0.0), (1000.0, 0.0)))["stats"]["segments_below_tol"] == 0
|
||||||
|
|
||||||
|
|
||||||
def test_a_branch_ending_on_a_header_is_a_t_junction():
|
def test_a_branch_ending_on_a_header_is_a_t_junction():
|
||||||
result = net(line("1", (0.0, 0.0), (2000.0, 0.0)), line("2", (1000.0, 0.0), (1000.0, 800.0)))
|
result = net(line("1", (0.0, 0.0), (2000.0, 0.0)), line("2", (1000.0, 0.0), (1000.0, 800.0)))
|
||||||
(run,) = result["runs"]
|
(run,) = result["runs"]
|
||||||
|
|||||||
@@ -37,6 +37,25 @@ def plant(tmp_path):
|
|||||||
return build_plant_pair(tmp_path)
|
return build_plant_pair(tmp_path)
|
||||||
|
|
||||||
|
|
||||||
|
async def test_a_metre_pid_keeps_a_pipe_shorter_than_one_drawing_unit(client, tmp_path):
|
||||||
|
# The junction tolerance defaults to 1 mm in the unit the P&ID's geometry
|
||||||
|
# implies. A fixed 1.0 would be a metre here: the 0.6 m pipe would collapse
|
||||||
|
# to a point and vanish from the totals.
|
||||||
|
import ezdxf
|
||||||
|
|
||||||
|
doc = ezdxf.new(setup=False)
|
||||||
|
doc.header["$INSUNITS"] = 6 # metres
|
||||||
|
msp = doc.modelspace()
|
||||||
|
msp.add_line((0.0, 0.0), (0.6, 0.0), dxfattribs={"layer": "PRODUCT"})
|
||||||
|
msp.add_line((10.0, 0.0), (15.0, 0.0), dxfattribs={"layer": "PRODUCT"})
|
||||||
|
msp.add_text("NOTES", height=0.25, dxfattribs={"layer": "TEXT"}).set_placement((0.0, 1.0))
|
||||||
|
path = tmp_path / "metre_pid.dxf"
|
||||||
|
doc.saveas(path)
|
||||||
|
result = (await client.call_tool("pipe_takeoff", {"pid_path": str(path)})).structured_content
|
||||||
|
assert result["units"]["pid"]["inferred"] == "m"
|
||||||
|
assert result["totals"]["net_m"] == pytest.approx(5.6) # 0.6 + 5.0
|
||||||
|
|
||||||
|
|
||||||
async def test_pipe_takeoff_measures_on_the_layout_and_writes_the_workbook(client, plant, tmp_path):
|
async def test_pipe_takeoff_measures_on_the_layout_and_writes_the_workbook(client, plant, tmp_path):
|
||||||
output = tmp_path / "takeoff.xlsx"
|
output = tmp_path / "takeoff.xlsx"
|
||||||
arguments = {
|
arguments = {
|
||||||
|
|||||||
Reference in New Issue
Block a user