API change in V1.2.0 breaks compatibility with downstream packages

[wtclarke]

Hi,

A recent change in the function definition of WeightingOp breaks compatibility with downstream packages.

LinearOperatorCollection.jl/src/WeightingOp.jl

Line 13 in 54bab32

function WeightingOp(weights::vecT, rep::Int=1) where {T <: Number, vecT<:AbstractVector{T}}

Prior to V1.2.0 it was defined as function WeightingOp(::Type{T}; weights::vecT, rep::Int=1) where {T <: Number, vecT<:AbstractVector} but is now missing a ";" in the definition function WeightingOp(weights::vecT, rep::Int=1) where {T <: Number, vecT<:AbstractVector{T}}. This means uses in e.g. MRIReco.jl throw an MethodError:

**ERROR:** LoadError: MethodError: no method matching WeightingOp(::Type{ComplexF32}; weights::Vector{ComplexF32}, rep::Int64)

Closest candidates are:

  WeightingOp(::vecT) where {T<:Number, vecT<:AbstractVector{T}} got unsupported keyword arguments "weights", "rep"

   @ LinearOperatorCollection ~/.julia/packages/LinearOperatorCollection/Ajsac/src/WeightingOp.jl:13

  WeightingOp(::vecT, ::Int64) where {T<:Number, vecT<:AbstractVector{T}} got unsupported keyword arguments "weights", "rep"

   @ LinearOperatorCollection ~/.julia/packages/LinearOperatorCollection/Ajsac/src/WeightingOp.jl:13

Stacktrace:

 [1] **reconstruction_multiCoilMultiEcho_subspace(**acqData::AcquisitionData{Float32, 3}, reconSize::Tuple{Int64, Int64, Int64}, reg::Vector{RegularizedLeastSquares.Regularization}, sparseTrafo::LinearOperator{ComplexF32, Int64, LinearOperatorWaveletExt.var"#3#6"{Tuple{Int64, Int64, Int64}, Wavelets.WT.OrthoFilter{Wavelets.WT.PerBoundary}}, Nothing, LinearOperatorWaveletExt.var"#4#7"{Tuple{Int64, Int64, Int64}, Wavelets.WT.OrthoFilter{Wavelets.WT.PerBoundary}}, Vector{ComplexF32}}, weights::Vector{Vector{ComplexF32}}, solvername::String, senseMaps::Array{ComplexF32, 4}, normalize::Bool, encodingOps::Nothing, params::Dict{Symbol, Any}**)**

   @ MRIReco ~/.julia/packages/MRIReco/rdhha/src/Reconstruction/IterativeReconstruction.jl:341

 [2] **reconstruction(**acqData::AcquisitionData{Float32, 3}, recoParams::Dict{Symbol, Any}**)**

   @ MRIReco ~/.julia/packages/MRIReco/rdhha/src/Reconstruction/Reconstruction.jl:51

Has this been introduced by mistake? Or should the developers of MRIReco.jl update their compatibility?

[nHackel]

Hello,

yes this was an intentional and breaking change. With the updated version number to 1.2 MRIReco.jl itself or rather MRIOperators.jl should not be affected by this as it is restricted to version 1.1.

Scripts and custom Julia environments that use MRIReco together with LinearOperatorCollection might need a compat entry for 1.1 in their Manifest.

I will at some point extend the changes to MRIReco and its subpackages too

[wtclarke]

OK, good to know and thanks for the reply. Then is the issue that MRIReco needs the version of LinearOperatorCollection (or MRIOperators if it's a chained dependency) pinned to v1.1.2 whilst the changes are waiting to be implemented in MRIReco?

[nHackel]

How did you encounter the error and with which version(s) of MRIReco and potentially its subpackages? MRIOperators is pinned to versions 1.1 as far as I can tell

Though there is actually an issue with the version number. Since we have a major version number a breaking change should have resulted in a version 2.X.

This is a seperate issue though which I will change when I update our upstream packages

[aTrotier]

We encountered this error trying to replicate one of my article figure : https://github.com/aTrotier/PAPER_subspace_MESE
where I used the following Project.toml files.

name = "Subspace_MESE"
uuid = "03ee4b65-1735-4169-82da-5df6ad43f66c"
authors = ["aTrotier <a.trotier@gmail.com> and contributors"]
version = "1.0.0"

[deps]
BartIO = "8b90e6a2-6e4d-4906-89ae-b5f72ca4a65b"
EPGsim = "b6a82cc1-40c9-4006-90b7-9176002c0410"
FFTW = "7a1cc6ca-52ef-59f5-83cd-3a7055c09341"
LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
LsqFit = "2fda8390-95c7-5789-9bda-21331edee243"
MRICoilSensitivities = "c57eb701-aafc-44a2-a53c-128049758959"
MRIFiles = "5a6f062f-bf45-497d-b654-ad17aae2a530"
MRIReco = "bdf86e05-2d2b-5731-a332-f3fe1f9e047f"

[compat]
julia = "1.9"
BartIO = "0.3.1"
EPGsim = "0.1.1"
FFTW = "1.7.2"
LsqFit = "0.15.0"
MRICoilSensitivities = "0.1.3"
MRIFiles = "0.2.0"
MRIReco = "0.8.1"

[extras]
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

[targets]
test = ["Test"]

Before, the 1.2.0 release (when I send my article for review), it was working :)

I'll add a compat entry to 1.1.2 but it might also be an issue for others package like KomaMRI that rely on MRIReco @cncastillo

[aTrotier]

It seems there is something weird with the compat entry...

Even if I put LinearOperatorCollection = "1.1.2" in my Project.toml, the version 1.2.0 is still used according to the manifest and I have to downgrad manually

[nHackel]

Ah okay then I have something to reproduce it with. I'm not 100% sure why it occurs, I though the package management shouldnt include it.

I'll try to take a look next week. Most of our software has versioning with 0.x atm and not X.Y so the compat might not be restrictive enough.

I might look into a fix where I roll back the changes, releae 1.3 and readd the changes and release 2.0. That way packages and scripts that always pull zhr latest 1.X release should work again

[aTrotier]

To reproduce the error, you can also start from scratch and ]add MRIOperators you will still end with version 1.2.0

[aTrotier]

Ok found the workaround :

I have to use

LinearOperatorCollection = "=1.1.2"

[cncastillo]

@aTrotier I believe this is not generating problems in Koma, we are using MRIReco "0.8.1" and the manifest says that we use LinearOperatorCollection "1.2.0". Thanks for letting me know 😄.

I believe the problem is what @nHackel is saying. In the compat "1.2.1" means "[1.2.1, 2.0)" which is different to "0.2.1" that means "[0.2.1, 0.3)" (Compat docs). The simplest solution is to register LinearOperatorCollection "1.2.1" which roll-backs to "1.1.X", and tag the new breaking release as "2.0".

Forcing package versions with Package = "=version" in the compat is not the best practice. I suggest setting LinearOperatorCollection = "1.1" in the Project.toml waiting for the fix, and downgrading the package manually with add LinearOperatorCollection@1.1.2 in the meantime.

[tknopp]

Yes, the issue is that one needs to adapt the thinking when switching from 0.x to 1.x. So from that point on the first number needs to change on breaking changes. I fear @nHackel that we need to do the fix, you proposed (we have some experience in that ;-) )

[nHackel]

Another fix we could try is adding a function which accepts the type and kwargs and just maps to the new interface.

It is question of what one considers a breaking change for the WeightingOp call. Previously we just constructed an opDiagonal from LinearOperators and didnt create a custom type. So the weights where "lost".

Now we construct a custom type which contains the weights and the opDiagonal. For certain solvers this allows efficient evaluation (for Kaczmarz/ART) because we can directly access the weights and for all other solvers things remain as they were.

So when we consider the concrete instance of the linear operator an implementation detail and just want the matrix-matrix/vector product to be the same we could consider this a non-breaking change and add the compatibility function. This would give us 1.2.1.

[wtclarke]

[like] William Clarke reacted to your message:

…

________________________________ From: Niklas Hackelberg ***@***.***> Sent: Tuesday, March 19, 2024 3:15:58 PM To: JuliaImageRecon/LinearOperatorCollection.jl ***@***.***> Cc: William Clarke ***@***.***>; Author ***@***.***> Subject: Re: [JuliaImageRecon/LinearOperatorCollection.jl] API change in V1.2.0 breaks compatibility with downstream packages (Issue #7) Closed #7<#7> as completed via 97e14da<97e14da>. — Reply to this email directly, view it on GitHub<#7 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AFJJTTTDI7R43TYEW4GSQ6DYZBJC5AVCNFSM6AAAAABER7LJAKVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJSGE3TCMRSHE3DENI>. You are receiving this because you authored the thread.Message ID: ***@***.***>