[Build] Add linters by hughperkins · Pull Request #6 · Genesis-Embodied-AI/quadrants

hughperkins · 2025-06-03T02:13:07Z

Issue: #

Brief Summary

Add linters

these are basically the same linters that upstream uses
but I'm running pre-commit from a script, rather than using precommit.ci, since precommit.ci doesn't work on private repos (at least, not without paying money; and it's so easy to simply run from a script anyway...)
for now it is missing clang-tidy. I need to figure out how to add that (upstream runs clang-tidy, so I can basically just copy that)
- I figure we can merge as-is, and then I can work on clang-tidy in parallel?

copilot:summary

Walkthrough

copilot:walkthrough

duburcqa · 2025-06-03T12:43:14Z

+      - uses: actions/checkout@v4
+      - name: Run linters
+        run: |
+          bash .github/workflows/scripts_new/linters.sh


Same again.

Ruff for instance has an official well maintained github action. I think it'd make sense to use it, so that what we have to maintain is minimal.

duburcqa · 2025-06-03T12:44:45Z

I figure we can merge as-is, and then I can work on clang-tidy in parallel?

Yes

…p/linters

CharlesMasson · 2025-06-05T13:44:28Z

+# Need to deal with C++ linkage issues first (and possibly
+# some other things), before we can turn on pyright
+
+pip install isort


What do you think of using ruff? We already use it in gs-core and it's the most robust and complete. We can only enable isort rules for now if we want to limit ourself to it.

I like ruff

I think we should merge all CI PRs first , before running a ruff pass, since that will conflict with the whole world...

(possibly excluding this one I suppose 🤔 )

Well... I suppose most of the changes are to either C++ or CI, which are both orthogonal to ruff 🤔

I think we could start using ruff now and only enable isort rules instead of migrating later, but up to you.

One advantage I see with ruff is that it has well-maintained pre-commit hooks and Github actions, so I think it'd allow removing some code in this PR, especially since it would also allow removing black and pylint in addition to isort.

oh interesting. didnt realize ruff check --select I was an option. Looking into that. Thanks!

Yes, but I'd favor configuring in pyproject.toml or ruff.toml, so that configuration is done in a single place and more transparently.

Yup, good point. thanks! 🙌

CharlesMasson · 2025-06-05T13:45:25Z

+      - uses: actions/checkout@v4
+      - name: Run linters
+        run: |
+          bash .github/workflows/scripts_new/linters.sh


Ruff for instance has an official well maintained github action. I think it'd make sense to use it, so that what we have to maintain is minimal.

…p/linters

CharlesMasson

See my comments about ruff, which I think would allow simplifying the CI and the pre-commit hooks (as it substitutes isort, black and pylint), and has a simple well-maintained pre-commit hook and a GitHub action that'd probably require less maintenance than the multiple tools. But either way, LGTM.

hughperkins · 2025-06-10T13:55:39Z

Thanks!

Introduce the helper machinery that the per-class to_torch / to_numpy methods will migrate to in subsequent commits. Existing public symbols (can_zerocopy, dlpack_to_torch, invalidate_zerocopy_cache, current_arch_is_cpu) are preserved as deprecated shims so the in-tree pre-rework callers continue to work; they will be removed once every call site is migrated. New surface: - _ZerocopyCache: per-instance container with two independent slots (torch tensor + numpy ndarray), each filled lazily on first access via torch.utils.dlpack.from_dlpack and numpy.from_dlpack respectively. Numpy zero-copy now bypasses torch entirely (closes review #6). - make_zerocopy_cache_if_supported(owner, ...): constructs a cache when zero-copy is supported and registers `owner` with `pyquadrants.cache_holders` so invalidation is wired automatically (closes review #18). - get_zerocopy_torch / get_zerocopy_numpy: thin entry points that implement the always-zerocopy-then-clone semantic (closes review #15, #16, #21) and the Apple Metal double-sync (qd.sync() on read AND torch.mps.synchronize() after .clone()/.to(); closes review #1, #22, #23). Also applies the small lints from the review: - Module-level constant for the torch>2.9.1 MPS bytes_offset probe; drops the pointless lru_cache wrapper around a zero-arg helper (closes review #2). - ASCII '...' instead of Unicode horizontal ellipsis '\u2026' in the docstring (closes review #3). - Top-level imports for numpy and torch (try/except for the no-torch CI case); no per-call lazy imports in the new code path (closes review #7, #9). The deprecated shim still does what the existing per-class methods expect; the new helpers are torch-clean. cache_holders is still empty until the next commits register Ndarray / ScalarField / MatrixField; this commit alone is no-op behaviourally.

Convert Ndarray.to_torch / _ndarray_to_numpy / _ndarray_matrix_to_numpy to thin calls into _interop.get_zerocopy_torch / get_zerocopy_numpy. Behaviour changes vs. the previous PR state: - to_torch(copy=True) now ALSO uses the zerocopy export and clones the result, instead of skipping zerocopy entirely. Zerocopy DLPack is cheaper than the kernel-copy fallback even when followed by a clone (closes review #15, #16, #21). - to_torch on Apple Metal now does both syncs: qd.sync() before the read AND torch.mps.synchronize() after the .clone() so the cloned buffer is actually populated by the time the call returns (closes review #1, #22, #23). - to_numpy(copy=False) now goes through numpy.from_dlpack directly -- no torch round-trip, no torch import required (closes review #6). - to_numpy default (copy=None) keeps the current "independent copy" semantics: numpy arrays conventionally outlive their source, so the default is the safer one. Zero-copy is opt-in via copy=False. Cache invalidation switches from the legacy _qd_dlpack_tc attribute to the per-class _zerocopy_cache (cached_property) + _invalidate_zerocopy_cache method. Registration in pyquadrants.cache_holders happens automatically on first cache access (closes review #18). The existing _reset hook drops its zerocopy invalidation call: that work is now handled by the cache_holders loop in impl.reset() BEFORE C++ teardown, eliminating the use-after-free that motivated the Genesis to_numpy() always-copy workaround.

Convert ScalarField.to_torch / to_numpy to thin calls into _interop.get_zerocopy_torch / get_zerocopy_numpy, mirroring the Ndarray migration in the previous commit. - Adds Field._invalidate_zerocopy_cache method (used by all subclasses) so the impl.reset() cache_holders loop has a single contract to call. - Adds ScalarField._zerocopy_cache as a cached_property; constructed once per instance instead of re-checking can_zerocopy on every call (closes review #17). - Registers ScalarField in pyquadrants.cache_holders on first cache access (closes review #18). - to_torch(copy=True) now zerocopies and clones rather than skipping zerocopy entirely (closes review #15, #16, #21); to_torch on Metal does both syncs (closes review #1, #22, #23). - to_numpy(copy=False) goes through numpy.from_dlpack directly, no torch round-trip and no torch import required (closes review #6). Default to_numpy semantic (copy=None) remains "independent copy" for lifetime safety; zero-copy is opt-in via copy=False.

Convert MatrixField.to_torch / to_numpy to thin calls into _interop.get_zerocopy_torch / get_zerocopy_numpy. Adds a single _matrix_view_shape() helper that returns (expected_shape, as_vector); both to_torch and to_numpy now go through it instead of duplicating the n / m / keep_dims branching (closes review #13, #14). Same wins as the ScalarField migration: - _zerocopy_cache as cached_property (closes review #17) - automatic registration with pyquadrants.cache_holders (closes #18) - copy=True now zerocopies + clones (closes #15, #16, #21) - to_numpy(copy=False) goes through numpy.from_dlpack directly, no torch round-trip (closes #6) - Apple Metal double-sync via the centralised helpers (closes #1, #22, #23) The redundant reshape after zerocopy stays (DLPack returns the field's flat n-d shape; numpy/torch users expect the matrix dims appended) but it's now in one place per method instead of two.

hughperkins added 2 commits June 2, 2025 19:10

linters

cbf1a03

pre-commit to use python 3.10

111f8e2

duburcqa reviewed Jun 3, 2025

View reviewed changes

hughperkins added 4 commits June 3, 2025 10:45

fix pyright issue on vtk.py

ffe8983

run pyright on modifeid files

0aed554

disalbe pyright

2d4e925

Merge branch 'main' into hp/linters

605a2da

duburcqa reviewed Jun 4, 2025

View reviewed changes

Comment thread python/taichi/tools/vtk.py

duburcqa approved these changes Jun 4, 2025

View reviewed changes

hughperkins added 3 commits June 4, 2025 13:36

Merge branch 'main' into hp/linters

65464bf

Merge branch 'hp/linters' of github.com:genesis-company/taichi into h…

d6412a9

…p/linters

fix linter error

bcb4126

CharlesMasson reviewed Jun 5, 2025

View reviewed changes

hughperkins added 3 commits June 5, 2025 07:03

Merge branch 'main' into hp/linters

349b356

Merge branch 'hp/linters' of github.com:genesis-company/taichi into h…

24d5723

…p/linters

pylint, black

9aeaceb

CharlesMasson approved these changes Jun 9, 2025

View reviewed changes

hughperkins added 5 commits June 9, 2025 17:16

Merge branch 'main' into hp/linters

faa15fe

run precommit

fbf76da

Linux x86 build

abe7ed1

fix vulkan url

f1098fb

name => Linters

c42ccf3

hughperkins merged commit bb2e85e into main Jun 10, 2025
15 of 16 checks passed

hughperkins deleted the hp/linters branch June 10, 2025 13:55

This was referenced Apr 16, 2026

[Perf] Tiles 4: add SharedArray slice support #482

Merged

[AutoDiff] Autodiff 10: Implement adstack for SPIR-V #490

Merged

[Perf] Enable zero-copy in to_torch() and to_numpy() #450

Merged

duburcqa mentioned this pull request Apr 24, 2026

[SPIRV] Vulkan SPIR-V correctness: atomic-view aliasing, PSB stride, narrow storage caps, u1 cast, per-init layer recheck #513

Merged

claude Bot mentioned this pull request May 1, 2026

[Perf] Streams 3: Add qd.stream_parallel() context manager #409

Open

Conversation

hughperkins commented Jun 3, 2025

Brief Summary

Walkthrough

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CharlesMasson Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

duburcqa commented Jun 3, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CharlesMasson Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CharlesMasson Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CharlesMasson left a comment

Choose a reason for hiding this comment

Uh oh!

hughperkins commented Jun 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

CharlesMasson Jun 5, 2025 •

edited

Loading

CharlesMasson Jun 9, 2025 •

edited

Loading

CharlesMasson Jun 5, 2025 •

edited

Loading