Skip to content

Conversation

ksagiyam
Copy link
Contributor

Cache the result of PointSetContext.basis_evaluation() on instance.

Currently, result of ctx.basis_evaluation(), where ctx = PointSetContext(), is cached on class, but the instance is not included in the cache key. This means that implementation changes around basis_evaluation() could cause hard-to-find caching bug. This PR makes the result cached on instance not on class.

We should note that the way we handle mt in the cache key generation is also fragile for the same reason.

@ksagiyam ksagiyam force-pushed the ksagiyam/cache_on_instance branch from 01b0086 to bb484ef Compare October 20, 2025 15:42
Co-authored-by: Leo Collins <[email protected]>

Co-authored-by: Pablo Brubeck <[email protected]>
@ksagiyam ksagiyam force-pushed the ksagiyam/cache_on_instance branch from 9798e57 to af33038 Compare October 21, 2025 09:49
@pbrubeck pbrubeck self-requested a review October 21, 2025 14:43
Copy link
Contributor

@pbrubeck pbrubeck left a comment

Choose a reason for hiding this comment

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

LGTM

@ksagiyam ksagiyam merged commit c0986bf into main Oct 21, 2025
6 of 7 checks passed
@ksagiyam ksagiyam deleted the ksagiyam/cache_on_instance branch October 21, 2025 14:55
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