_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

Nobody has claimed this yet.

Assessment

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

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.

Description

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

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).

Dominant language
Python
Stars
11
Forks
6
Avg merge
8m
Merged PRs (30d)
4

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from FireDynamics/fdsvismap

All issues in FireDynamics/fdsvismap

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.