FireDynamics / FireDynamics/fdsvismap

_add_visual_object snaps the y-axis with the x cell size

Open Beginner friendly
#44 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
11
Forks
6
Avg merge
8m
Merged PRs (30d)
4

Description

`fdsvismap/FDSVisMap.py:1126,1129` (current `feat/synthetic-clear-air-grid`; same on `v.0.2.1`):

```python
ref_y1_id = get_id_of_closest_value(
self.all_y_coords, y1 + self.cell_size[0] / 2 # <- [0] is the x size
)
ref_y2_id = (
get_id_of_closest_value(self.all_y_coords, y2 - self.cell_size[0] / 2) + 1
)
```

Both y snaps use `cell_size[0]`; they should use `cell_size[1]`. Harmless whenever dx == dy — which is every current test and the square grids `set_grid` callers use — but on an anisotropic mesh (perfectly legal in FDS) obstruction rectangles snap to the wrong rows: with dx=1.0, dy=0.25 the half-cell inset applied to y is 4x too large, so a thin obstruction can lose its edge rows or vanish.

Latent today, but #41's `set_grid` makes anisotropic grids a one-line call, so the exposure grows. Two-character fix plus one test on a dx != dy grid asserting a known cell pattern.

Found while reviewing #41; deliberately not folded into it (pre-existing, and its tests all use square cells so it would ride in unverified).

Contributor guide

No contributing guide indexed for this repository

Research direction

Start at fdsvismap/FDSVisMap.py lines 1126 and 1129, where _add_visual_object snaps the y coordinates, and review the existing grid-related tests. Replace the y-axis cell-size reference and add a regression test using dx != dy that asserts the expected cell pattern. Run the existing test suite to confirm square-grid behavior remains unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
tooling
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
84/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.