Skip to content

Add standalone module internals.weak with data structures for caching - #744

Merged
jfeser merged 6 commits into
masterfrom
eb-weak-module
Aug 10, 2026
Merged

Add standalone module internals.weak with data structures for caching#744
jfeser merged 6 commits into
masterfrom
eb-weak-module

Conversation

@eb8680

@eb8680 eb8680 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Factored out of #743 to ease review.

This PR contains only the new internal-facing module effectful.internals.weak and its tests tests/test_internals_weak.py from #743. This module and its tests are self-contained and do not depend on any other part of effectful; no changes here affect the rest of the library in any way.

Most of the implementation of internals.weak.WeakIdKeyDictionary is copied from torch.utils.weak, although I've made some fixes and cleaned up types and docstrings. The things on top of that are new.

@jfeser jfeser left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've pushed a failing test that should pass. I haven't read this code in detail, just put it through GPT.

Comment thread tests/test_internals_weak.py Outdated
@eb8680

eb8680 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

I fixed the test you pushed.

I haven't read this code in detail, just put it through GPT.

The nastiest part of the code is the implementation of WeakIdKeyDictionary (including _iterationGuard and WeakIdRef) which is a forked and lightly modified version of torch.utils.weak.WeakIdKeyDictionary and thus shouldn't need careful review. The other stuff in weak.py is more straightforward and largely sits on top of that. The tests were generated by Claude from a specification I wrote and reviewed with/for it, along with the tests from PyTorch for torch.utils.weak (which are not exactly comprehensive). I am never thrilled with Claude's test code but I think it covered everything in the spec.

Comment thread effectful/internals/weak.py Outdated
@jfeser
jfeser self-requested a review August 10, 2026 16:15
@jfeser
jfeser merged commit 79554e0 into master Aug 10, 2026
29 checks passed
@jfeser
jfeser deleted the eb-weak-module branch August 10, 2026 16:39
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.

2 participants