Skip to content

Deprecate automatic flattening for tensor products#129

Merged
inducer merged 2 commits into
inducer:mainfrom
alexfikl:remove-tensor-product-flatten
Jun 26, 2026
Merged

Deprecate automatic flattening for tensor products#129
inducer merged 2 commits into
inducer:mainfrom
alexfikl:remove-tensor-product-flatten

Conversation

@alexfikl

Copy link
Copy Markdown
Collaborator

This adds a .flatten() method to tensor products to recursively flatten them into simplices (mostly). The automatic flattening in the constructor is marked as deprecated.

Fixes #126.

@inducer

inducer commented Jul 28, 2025

Copy link
Copy Markdown
Owner

I'm supportive of this change. How come it's marked as draft?

@alexfikl

Copy link
Copy Markdown
Collaborator Author

I'm supportive of this change. How come it's marked as draft?

Wasn't quite sure about adding that flatten: bool = True argument and outright deleting the custom __new__ implementation. Doesn't seem to break anything though, so should at least be good for a quick review 😁

@alexfikl alexfikl force-pushed the remove-tensor-product-flatten branch 3 times, most recently from 37ea45f to a27cd45 Compare July 30, 2025 07:14
@alexfikl alexfikl force-pushed the remove-tensor-product-flatten branch 3 times, most recently from 43887f3 to d46952b Compare June 25, 2026 06:46
@alexfikl alexfikl requested a review from Copilot June 25, 2026 06:47

Copilot AI 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.

Pull request overview

This PR updates ModePy’s tensor-product abstractions to stop implicitly “simplifying” tensor products while providing an explicit .flatten() API, and begins deprecating constructor-time auto-flattening to address surprising behavior reported in #126.

Changes:

  • Add .flatten() methods for TensorProductSpace and TensorProductShape to recursively flatten nested tensor products (returning the single remaining base directly).
  • Replace __new__-based auto-simplification with constructor-time behavior guarded by a flatten flag and DeprecationWarning when auto-flattening is applied.
  • Adjust hypercube-related implementations to avoid relying on 1D hypercubes being Simplex, and update tests/type-check baselines accordingly.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
modepy/spaces.py Adds flatten constructor flag, emits deprecation warnings for auto-flattening, and introduces TensorProductSpace.flatten().
modepy/shapes.py Adds flatten constructor flag + TensorProductShape.flatten(), updates face counting, and removes 1D hypercube “becomes simplex” behavior.
modepy/test/test_tools.py Removes assertions that depended on 1D tensor products/hypercubes collapsing to simplices.
.basedpyright/baseline.json Removes baseline entries related to the deleted overload-based __new__ implementations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread modepy/spaces.py
Comment thread modepy/spaces.py
Comment thread modepy/spaces.py Outdated
Comment thread modepy/shapes.py
@alexfikl alexfikl force-pushed the remove-tensor-product-flatten branch from d46952b to dc4093e Compare June 25, 2026 07:10
@alexfikl alexfikl marked this pull request as ready for review June 26, 2026 14:24
@inducer inducer force-pushed the remove-tensor-product-flatten branch from dc4093e to 4fc9ba0 Compare June 26, 2026 14:40
@inducer inducer enabled auto-merge (rebase) June 26, 2026 14:43
@inducer inducer merged commit 85bb83d into inducer:main Jun 26, 2026
9 checks passed
@inducer

inducer commented Jun 26, 2026

Copy link
Copy Markdown
Owner

Thanks!

@alexfikl alexfikl deleted the remove-tensor-product-flatten branch June 26, 2026 16:41
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.

Stop auto-flattening and simplifying TensorProducts

3 participants