Closed apkille closed 2 months ago
Attention: Patch coverage is 88.88889%
with 7 lines
in your changes missing coverage. Please review.
Project coverage is 65.86%. Comparing base (
237d158
) to head (076399a
).
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
A day or two ago I spent some time to make sure that all tests pass across this github organization, so I will be a bit more strict on tests during review. Here is a list of explanations:
The macos error was just a random happenstance
The codecov coverage issue is not great, but not a blocker
The performance tracking is not set up yet, that is why it is failing
The changelog will be updated on merging
The only problematic one is the JET tests. JET is a static analyzer, i.e. it attempts to detect bugs without running the code, i.e. logic bugs that would be pretty difficult to detect when running the code anyway because they might be in an edge case not covered by tests and rarely encountered by users. It does have false positives though, so I have simply upper bounded the number of errors it is permitted to detect. It was at 6, but now it detects 7 issues. Could you check if any of these issues were caused by something changed in this PR. I copy them here for convenience:
rep = ═════ 7 possible errors found ═════
┌ commutator(o1::SymbolicUtils.Symbolic{…}, o2::SymbolicUtils.Symbolic{…}) @ QuantumSymbolics /home/runner/work/QuantumSymbolics.jl/QuantumSymbolics.jl/src/QSymbolicsBase/basic_ops_homogeneous.jl:209
│┌ QuantumSymbolics.SCommutator(o1::SymbolicUtils.Symbolic{…}, o2::SymbolicUtils.Symbolic{…}) @ QuantumSymbolics /home/runner/work/QuantumSymbolics.jl/QuantumSymbolics.jl/src/QSymbolicsBase/basic_ops_homogeneous.jl:199
││┌ hcat(::SymbolicUtils.Symbolic{…}, ::SymbolicUtils.Symbolic{…}) @ Base ./abstractarray.jl:1944
│││┌ kwcall(::@NamedTuple{…}, ::typeof(cat), ::SymbolicUtils.Symbolic{…}, ::SymbolicUtils.Symbolic{…}) @ Base ./abstractarray.jl:1993
││││┌ cat(::SymbolicUtils.Symbolic{…}, ::SymbolicUtils.Symbolic{…}; dims::Val{…}) @ Base ./abstractarray.jl:1993
│││││┌ _cat(::Val{…}, ::SymbolicUtils.Symbolic{…}, ::SymbolicUtils.Symbolic{…}) @ Base ./abstractarray.jl:1796
││││││┌ _cat_t(::Val{…}, ::Type{…}, ::SymbolicUtils.Symbolic{…}, ::SymbolicUtils.Symbolic{…}) where T @ Base ./abstractarray.jl:1805
│││││││┌ __cat(::Matrix, ::Tuple{…}, ::Tuple{…}, ::SymbolicUtils.Symbolic{…}, ::SymbolicUtils.Symbolic{…}) @ Base ./abstractarray.jl:1812
││││││││┌ __cat_offset!(A::Matrix, shape::Tuple{…}, catdims::Tuple{…}, offsets::Tuple{…}, x::SymbolicUtils.Symbolic{…}, X::SymbolicUtils.Symbolic{…}) @ Base ./abstractarray.jl:1816
│││││││││┌ __cat_offset1!(A::Matrix, shape::Tuple{…}, catdims::Tuple{…}, offsets::Tuple{…}, x::SymbolicUtils.Symbolic{…}) @ Base ./abstractarray.jl:1825
││││││││││┌ _copy_or_fill!(A::Matrix, inds::Tuple{…}, x::SymbolicUtils.Symbolic{…}) @ Base ./abstractarray.jl:1832
│││││││││││┌ fill!(V::SubArray{Bool, 2, Union{}, Tuple{…}, false}, x::SymbolicUtils.Symbolic{QuantumInterface.AbstractOperator}) @ Base ./multidimensional.jl:1512
││││││││││││┌ getproperty(x::SubArray{Bool, 2, Union{}, Tuple{UnitRange{Int64}, UnitRange{Int64}}, false}, f::Symbol) @ Base ./Base.jl:37
│││││││││││││ invalid builtin function call: Base.getfield(x::SubArray{Bool, 2, Union{}, Tuple{UnitRange{Int64}, UnitRange{Int64}}, false}, f::Symbol)
││││││││││││└────────────────────
│││││││││││┌ fill!(V::SubArray{…}, x::SymbolicUtils.Symbolic{…}) where Tv @ SparseArrays /cache/build/builder-amdci4-0/julialang/julia-release-1-dot-10/usr/share/julia/stdlib/v1.10/SparseArrays/src/sparsematrix.jl:3168
││││││││││││┌ getproperty(x::SubArray{Tv, 2, Union{}, Tuple{UnitRange{Int64}, UnitRange{Int64}}, false} where Tv, f::Symbol) @ Base ./Base.jl:37
│││││││││││││ invalid builtin function call: Base.getfield(x::SubArray{Tv, 2, Union{}, Tuple{UnitRange{Int64}, UnitRange{Int64}}, false} where Tv, f::Symbol)
││││││││││││└────────────────────
┌ @ QuantumSymbolics /home/runner/work/QuantumSymbolics.jl/QuantumSymbolics.jl/src/QSymbolicsBase/predefined.jl:314
│ no matching method found `QuantumSymbolics.SMulOperator(::QuantumSymbolics.SInvOperator, ::QuantumSymbolics.SOperator)`: SMulOperator(invop::QuantumSymbolics.SInvOperator, op::QuantumSymbolics.SOperator)
└────────────────────
┌ @ QuantumSymbolics /home/runner/work/QuantumSymbolics.jl/QuantumSymbolics.jl/src/QSymbolicsBase/predefined.jl:315
│ no matching method found `QuantumSymbolics.SMulOperator(::QuantumSymbolics.SOperator, ::QuantumSymbolics.SInvOperator)`: SMulOperator(op::QuantumSymbolics.SOperator, invop::QuantumSymbolics.SInvOperator)
└────────────────────
┌ (::QuantumSymbolics.var"#43#45")(__MATCHES__::Any) @ QuantumSymbolics /home/runner/.julia/packages/SymbolicUtils/0opve/src/rule.jl:314
│ `QuantumSymbolics.xs` is not defined: QuantumSymbolics.prefactorscalings_rule(QuantumSymbolics.xs)
└────────────────────
┌ apply_recipe(x::QuantumSymbolics.SDagger; kwargs...) @ QuantumSymbolics /home/runner/work/QuantumSymbolics.jl/QuantumSymbolics.jl/src/QSymbolicsBase/latexify.jl:31
│┌ getproperty(x::QuantumSymbolics.SDagger, f::Symbol) @ Base ./Base.jl:37
││ type SDagger has no field ket: Base.getfield(x::QuantumSymbolics.SDagger, f::Symbol)
│└────────────────────
┌ basis(x::QuantumSymbolics.StabilizerState) @ QuantumSymbolics /home/runner/work/QuantumSymbolics.jl/QuantumSymbolics.jl/src/extensions.jl:25
│┌ ^(b::SpinBasis{1//2, Int64}, N::Integer) @ QuantumInterface /home/runner/.julia/packages/QuantumInterface/VISSI/src/bases.jl:100
││┌ collect(itr::Base.Generator{_A, QuantumInterface.var"#3#4"{SpinBasis{1//2, Int64}}} where _A) @ Base ./array.jl:844
│││┌ collect_to_with_first!(dest::Any, v1::SpinBasis{…}, itr::Base.Generator{…} where _A, st::Any) @ Base ./array.jl:875
││││┌ grow_to!(dest::AbstractDict{…}, itr::Base.Generator{…} where _A, st::Any) where {…} @ Base ./dict.jl:133
│││││┌ indexed_iterate(I::SpinBasis{1//2, Int64}, i::Int64) @ Base ./tuple.jl:95
││││││ no matching method found `iterate(::SpinBasis{1//2, Int64})`: x = iterate(I::SpinBasis{1//2, Int64})
│││││└────────────────────
actually, it seems the JET tests fail on master too, because of something I must have merged last minute as I was cleaning up the tests. You can just bump the threshold to 7 and not bother with them
Looks good, feel free to merge after you add a changelog and all tests except "performance tracking" and "codecov project" pass. Some more details on merging etiquette were posted in the other PR
Added zero structs (#46) and created a hashing method for expressions containing quantum objects #47 in order to define expression equality for commutative operations.