Skip to content

Conversation

@AFeuerpfeil
Copy link
Contributor

In this PR, I change the PeriodicArray type, such that it can be used on any AbstractArray.
This has the advantage that the user can supply a suitable array for special applications such as MPI parallelization.

One major downside is that now PeriodicVector{O} is not concrete anymore. One could circumvent this in the code by replacing all PeriodicVector{O} by PeriodicVector{O,A} where {A<:AbstractVector{O}} where {O} and types such as InfiniteMPS to carry the typeinformation A<:AbstractVector{O} as a type parameter.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2025

Your PR requires formatting changes to meet the project's style guidelines.
Please consider running Runic (git runic main) to apply these changes.

Click here to view the suggested changes.
diff --git a/src/utility/periodicarray.jl b/src/utility/periodicarray.jl
index 867c714..32e0dc2 100644
--- a/src/utility/periodicarray.jl
+++ b/src/utility/periodicarray.jl
@@ -28,13 +28,11 @@ See also [`PeriodicVector`](@ref), [`PeriodicMatrix`](@ref)
 """
 struct PeriodicArray{T, N, A <: AbstractArray{T, N}} <: AbstractArray{T, N}
     data::A
-    PeriodicArray{T,N}(data::A) where A <: AbstractArray{T,N} where {T,N} = new{T,N,A}(data)
-    PeriodicArray{T,N,A}(data::A) where A <: AbstractArray{T,N} where {T,N} = new{T,N,A}(data)
+    PeriodicArray{T, N}(data::A) where {A <: AbstractArray{T, N}} where {T, N} = new{T, N, A}(data)
+    PeriodicArray{T, N, A}(data::A) where {A <: AbstractArray{T, N}} where {T, N} = new{T, N, A}(data)
 end
-PeriodicArray(data::AbstractArray{T,N}) where {T,N} = PeriodicArray{T,N}(data)
-PeriodicArray{T}(data::AbstractArray{T,N}) where {T,N} = PeriodicArray{T,N}(data)
-
-
+PeriodicArray(data::AbstractArray{T, N}) where {T, N} = PeriodicArray{T, N}(data)
+PeriodicArray{T}(data::AbstractArray{T, N}) where {T, N} = PeriodicArray{T, N}(data)
 
 
 function PeriodicArray{T}(initializer, args...) where {T}

@lkdvos
Copy link
Member

lkdvos commented Nov 7, 2025

I am not necessarily opposed to this, but as you pointed out (and as the tests show), this is very much a breaking change, that will affect all users and downstream packages in quite a non-trivial way.

One subtle part about this breaking change is IIRC, there are actually some parts where we are making use of the fact that the PeriodicArray canonicalizes the incoming storage type. For example, currently there is no danger in PeriodicArray(view(A, 1:3)) sharing data, while with the new design this would no longer be the case.

Additionally, there is the question about if we should generalize at that level, or simply generalize the storage type of the InfiniteMPS themselves. I'm still not entirely convinced this is the right strategy for dealing with your problem, but I'd love to hear more opinions, e.g. from @VictorVanthilt or @leburgel ?

@lkdvos lkdvos closed this Nov 10, 2025
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.

2 participants