Skip to content

Specialize norm(t, Inf) a bit so we can use it on GPU backed TensorMaps - #586

Open
kshyatt wants to merge 5 commits into
mainfrom
ksh/norminf
Open

kshyatt wants to merge 5 commits into
mainfrom
ksh/norminf

Conversation

@kshyatt

@kshyatt kshyatt commented Oct 19, 2026

Copy link
Copy Markdown
Member

This should give us normalize!(t, Inf) for free as well. The issue right now is the LinearAlgbra methods cause scalar indexing errors due to the checks they'll do on views, but for this specific case we can go directly to the underlying data array.

@kshyatt
kshyatt requested a review from lkdvos October 19, 2026 14:40
Comment thread src/tensors/linalg.jl Outdated
@codecov

codecov Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
src/tensors/adjoint.jl 88.57% <100.00%> (+1.47%) ⬆️
src/tensors/diagonal.jl 90.41% <100.00%> (+0.22%) ⬆️
src/tensors/linalg.jl 84.11% <100.00%> (+0.73%) ⬆️

... and 14 files with indirect coverage changes

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

@kshyatt
kshyatt marked this pull request as ready for review October 20, 2026 06:51
Comment thread src/tensors/adjoint.jl Outdated
Adapt.adapt_structure(to, x::AdjointTensorMap) = adjoint(Adapt.adapt(to, parent(x)))

function LinearAlgebra.norm(t::AdjointTensorMap, p::Real = 2)
InnerProductStyle(t) === EuclideanInnerProduct() || throw_invalid_innerproduct(:norm)

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.

Not really related to this PR, but I am confused how we ever ended up in this situation. The InnerProductStyle check only makes sense if we compute the norm with p==2. For all other values of p, this norm has nothing to do with the inner product anyway, so why should we check. Then again, for all other values of p, this norm doesn't really have a clear physical meaning; it depends on the chosen way to store the tensor data. In particular, norm(t, p) ≈ norm(convert(Array, t), p) will only hold for p == 2.

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.

Related: #548, I would still be very happy to simply remove most of this, it adds quite a bit of boilerplate that is not really usable

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Should I then leave this in place, for now?

Comment thread src/tensors/linalg.jl Outdated
Comment thread src/tensors/adjoint.jl Outdated

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants