Visitar URL original
Fix higher_order_moment in the core and einsum tenalg backends by raashish1601 · Pull Request #650 · tensorly/tensorly · GitHub
Skip to content

Fix higher_order_moment in the core and einsum tenalg backends - #650

Open
raashish1601 wants to merge 2 commits into
tensorly:mainfrom
raashish1601:fix/higher-order-moment
Open

raashish1601 wants to merge 2 commits into
tensorly:mainfrom
raashish1601:fix/higher-order-moment

Conversation

@raashish1601

Copy link
Copy Markdown
Contributor

tl.tenalg.higher_order_moment fails with both tenalg backends:

import numpy as np, tensorly as tl
x = np.random.rand(6, 3)
tl.tenalg.higher_order_moment(x, 3)
# core:   TypeError: batched_outer() takes 1 positional argument but 2 were given
# einsum: ValueError: einstein sum subscripts string includes output subscript 'c' multiple times
  • core: batched_outer takes a list of tensors since the batched-tensordot API change, but higher_order_moment still called it with two arguments. Now it passes [moment, tensor].
  • einsum: the equation used a single operand and repeated the feature subscripts in the output ("bc->cc..."). It now uses order copies of the tensor, each with its own feature subscripts and a shared batch subscript, and divides by the number of samples, e.g. "ab,ac,ad->bcd" for a matrix and order=3.

There were no tests for this function, so I added tensorly/tenalg/tests/test_moments.py. It compares against np.einsum for a matrix (orders 1 and 3) and for samples that are tensors themselves (order 2). The test fails on main with both backends and passes with this change. I also ran pytest tensorly/tenalg with TENSORLY_TENALG_BACKEND=core and =einsum (284 passed each) and black --check.

The core version passed two tensors to batched_outer, which takes a
list, so it raised a TypeError. The einsum version built an equation
with a single operand and repeated output subscripts, so it raised a
ValueError. Build the moment from order copies of the tensor in both
backends and add tests.
@codecov

codecov Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 63.15789% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.10%. Comparing base (bbd985b) to head (2012a64).
⚠️ Report is 65 commits behind head on main.

Files with missing lines Patch % Lines
tensorly/tenalg/einsum_tenalg/moments.py 0.00% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #650      +/-   ##
==========================================
+ Coverage   90.71%   91.10%   +0.39%     
==========================================
  Files         132      133       +1     
  Lines        8056     8828     +772     
==========================================
+ Hits         7308     8043     +735     
- Misses        748      785      +37     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JeanKossaifi JeanKossaifi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, looks good, just some minor naming comment to make the code clearer.

Co-authored-by: Jean Kossaifi <jean.kossaifi@gmail.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants