Skip to content

Default spacetype and sectortype implementations in type domain#248

Merged
lkdvos merged 3 commits intoQuantumKitHub:masterfrom
ogauthe:spacetype
Jun 17, 2025
Merged

Default spacetype and sectortype implementations in type domain#248
lkdvos merged 3 commits intoQuantumKitHub:masterfrom
ogauthe:spacetype

Conversation

@ogauthe
Copy link
Contributor

@ogauthe ogauthe commented Jun 16, 2025

PEPSKit.jl overload sectortype and spacetype for many objects (see QuantumKitHub/PEPSKit.jl@af3f140 or QuantumKitHub/PEPSKit.jl#218). Currently one has to repeat 4 lines of code for every struct in order to dispatch on type and use sectortype(spacetype).

This PR proposes to set defaults in TensorKit such that a single line

spacetype(::Type{MyType}) = ...

is sufficient to use sectortype(x::MyType) and spacetype(x::MyType).

I also defined a default for field as it is very similar. I kept the specializations for AbstractTensorMap to conserve easy access to the documentation, I do not know if there is a better way.

@lkdvos
Copy link
Member

lkdvos commented Jun 16, 2025

As we discussed, the docs can perhaps be migrated to a centralized location through:

@doc """
...
""" spacetype

Otherwise definitely looks like a nice quality of life addition to me, also relevant for MPSKit.

@ogauthe
Copy link
Contributor Author

ogauthe commented Jun 16, 2025

I generalized the doc. Also generalized sectortype to allow sectortype(SU2Irrep) == SU2Irrep.

@codecov
Copy link

codecov bot commented Jun 16, 2025

Codecov Report

Attention: Patch coverage is 70.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 82.66%. Comparing base (0c73bd2) to head (2f46fdd).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/spaces/vectorspaces.jl 70.00% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #248   +/-   ##
=======================================
  Coverage   82.65%   82.66%           
=======================================
  Files          43       43           
  Lines        5593     5583   -10     
=======================================
- Hits         4623     4615    -8     
+ Misses        970      968    -2     

☔ View full report in Codecov by Sentry.
📢 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.

Copy link
Member

@lkdvos lkdvos left a comment

Choose a reason for hiding this comment

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

I definitely think this is a QOL improvement, @Jutho how do you feel about this?

Thanks @ogauthe !

@Jutho
Copy link
Member

Jutho commented Jun 16, 2025

Yes this is indeed much more convenient. The only small comment is that I would streamline:

sectortype(::Type{S}) where {S<:Sector} = S
spacetype(S::Type{<:ElementarySpace}) = S

so that they take the same form (both seem to do the job).

@ogauthe
Copy link
Contributor Author

ogauthe commented Jun 16, 2025

Yes this is indeed much more convenient. The only small comment is that I would streamline:

sectortype(::Type{S}) where {S<:Sector} = S
spacetype(S::Type{<:ElementarySpace}) = S

so that they take the same form (both seem to do the job).

I found there are subtle differences in dispatch behavior between these 2 versions (see https://docs.julialang.org/en/v1/manual/performance-tips/#Be-aware-of-when-Julia-avoids-specializing). I now impose specialization.

@lkdvos
Copy link
Member

lkdvos commented Jun 16, 2025

This is true, but often not relevant since these functions get inlined and constant folded most of the time, but I fully agree not relying on these implicit compiler details is annoying since it's hard to verify, so might as well be explicit about it

@lkdvos lkdvos changed the title generic spacetype and sectortype Default spacetype and sectortype implementations in type domain Jun 16, 2025
@lkdvos lkdvos merged commit c37cf86 into QuantumKitHub:master Jun 17, 2025
10 of 13 checks passed
@ogauthe ogauthe deleted the spacetype branch June 17, 2025 13:42
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