Skip to content

[Build] Fix tools test on wheel#17

Merged
hughperkins merged 5 commits intomainfrom
hp/fix-tools-test-on-wheel
Jun 9, 2025
Merged

[Build] Fix tools test on wheel#17
hughperkins merged 5 commits intomainfrom
hp/fix-tools-test-on-wheel

Conversation

@hughperkins
Copy link
Copy Markdown
Collaborator

Issue: #

Brief Summary

Fix tools test on wheel

copilot:summary

Walkthrough

copilot:walkthrough

Comment on lines +8 to +13
# I don't want to make python/tools a module, and I don't want to move this tool
# into `taichi` namespace, so that leaves temporarily importing it somehow
tools_path = Path(__file__).parent.parent.parent / "python" / "tools" / "markdown_link_check.py"
spec = importlib.util.spec_from_file_location("markdown_link_check", tools_path)
markdown_link_check = importlib.util.module_from_spec(spec)
spec.loader.exec_module(markdown_link_check)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Isn't there any way we can solve this declaratively in pyproject.toml?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Well, yes, by putting this into taichi namespace, but this is strictly for use in CI, not part of Taichi product, so I'm not very keen on doing that. I suppose we could make a new namespace, like taichi-ci perhaps 🤔 Thoughts?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

(I'm not incredibly keen on adding a new taichi-ci namespace to be honest. we could also have taichi.ci I suppose 🤔 )

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

(unless you're aware of some way of handling this in pyproject.toml , that does not involve either adding this to taichi namespace or similar?)

Copy link
Copy Markdown
Contributor

@duburcqa duburcqa left a comment

Choose a reason for hiding this comment

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

This is ugly but this part of the code is not critical so it is fine for me.

@hughperkins
Copy link
Copy Markdown
Collaborator Author

Thanks!

@hughperkins hughperkins merged commit f7b5d01 into main Jun 9, 2025
14 checks passed
@hughperkins hughperkins deleted the hp/fix-tools-test-on-wheel branch June 9, 2025 15:26
hughperkins added a commit that referenced this pull request Apr 26, 2026
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.
hughperkins added a commit that referenced this pull request Apr 26, 2026
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.
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.

3 participants