Skip to content

Add the terrain store: region grids and tile cache - #55

Open
mouftz wants to merge 1 commit into
mainfrom
store-region-grids
Open

Add the terrain store: region grids and tile cache#55
mouftz wants to merge 1 commit into
mainfrom
store-region-grids

Conversation

@mouftz

@mouftz mouftz commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements the Terrain Store (#31) — all six sub-tickets in one PR, since
the two halves are small and share a module.

Adds numpy (2.5.2) as the project's first runtime dependency, per
CONTRIBUTING: pyproject.toml and uv.lock are both committed.

Validation

  • scripts/quality-check.sh passes locally
  • Appropriate /test commands were run and are passing (e.g. /test gpu)
  • Unit tests were added / e2e tests were added where applicable
  • Manual testing, if applicable (describe further):

27 tests in tests/test_store.py, covering every assertion listed in
#40#45. The overlap test uses the worked 1×6 example from #31 directly.
Full suite is 68 passed.

Checklist

  • The change is focused and easy to review.
  • Tests were added or updated for behaviour changes.
  • No secrets, generated files, or files that only make sense on my machine are included.
  • Relevant documentation is updated.

Additional Comments

Two decisions worth a second opinion:

  • put copies the incoming array so the generator can reuse its buffer,
    but get hands back the cache's own grid rather than a copy — copying
    on every read would undo the point of caching. Callers should treat a
    returned tile as read-only. Happy to change this if you would rather it
    be defensive on both sides.
  • Disk persistence is mentioned in the module docstring but has no
    sub-ticket, so it is not implemented here.

Closes #40, #41, #42, #43, #44, #45

@mouftz mouftz self-assigned this Aug 23, 2026
@coveralls

coveralls commented Aug 23, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 32647543760

Coverage increased (+1.0%) to 97.744%

Details

  • Coverage increased (+1.0%) from the base build.
  • Patch coverage: 1 uncovered change across 1 file (191 of 192 lines covered, 99.48%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
src/terrain_diffusion/store.py 56 55 98.21%
Total (2 files) 192 191 99.48%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 532
Covered Lines: 520
Line Coverage: 97.74%
Coverage Strength: 0.98 hits per line

💛 - Coveralls

@mouftz
mouftz force-pushed the store-region-grids branch from d974f1c to 1ea3053 Compare August 23, 2026 15:07
@mouftz
mouftz requested a review from KurbyDoo August 23, 2026 18:28

@KurbyDoo KurbyDoo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed some of the specifications of the original tickets since they don't really match what I'm thinking anymore, could you update this PR accordingly? Thanks

import numpy as np


class RegionGrids:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry I'm looking back at the original paper and it doesn't look like there are two different objects to store region grids and tile cache, can we merge the two into one. the only thing that should be cached is the weight value product and the weights as two seperate grids

"""Whether a tile is held, without counting as a use of it."""
return seed in self._tiles

def get(self, seed: int) -> np.ndarray | None:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also think caching by seed is no longer best, can we instead cache by tile x, y coordinates. you can assume that every tile will live at a location at some x and y in a global coordinate space. in general tiles divide the global space into a partition with no overlap

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create empty sum and weight grids for a region

3 participants