JuliaMolSim / Molly.jl

Molecular simulation in Julia
Other
393 stars 53 forks source link

testing failure with CellListMap v0.8.3 #102

Closed leios closed 2 years ago

leios commented 2 years ago

I updated CellListMap v0.7.21 ⇒ v0.8.3 and received the following errors on testing the master branch.

Neighbor lists: Error During Test at /home/leios/projects/CESMIX/Molly.jl/test/basic.jl:117
  Got exception outside of a @test
  ArgumentError: zero(Quantity{Float64}) not defined.
  Stacktrace:
    [1] zero(x::Type{Quantity{Float64}})
      @ Unitful ~/.julia/packages/Unitful/ApCuY/src/quantities.jl:389
    [2] (::CellListMap.var"#95#96"{SMatrix{3, 3, Quantity{Float64}, 9}})(el::Quantity{Float64, 𝐋, Unitful.FreeUnits{(nm,), 𝐋, nothing}})
      @ CellListMap ~/.julia/packages/CellListMap/rqPY5/src/CellOperations.jl:547
    [3] (::StaticArrays.var"#194#195"{CellListMap.var"#95#96"{SMatrix{3, 3, Quantity{Float64}, 9}}})(x::Quantity{Float64, 𝐋, Unitful.FreeUnits{(nm,), 𝐋, nothing}})
      @ StaticArrays ~/.julia/packages/StaticArrays/atiPw/src/mapreduce.jl:259
    [4] macro expansion
      @ ~/.julia/packages/StaticArrays/atiPw/src/mapreduce.jl:140 [inlined]
    [5] _mapfoldl
      @ ~/.julia/packages/StaticArrays/atiPw/src/mapreduce.jl:115 [inlined]
    [6] _mapreduce
      @ ~/.julia/packages/StaticArrays/atiPw/src/mapreduce.jl:113 [inlined]
    [7] #count#193
      @ ~/.julia/packages/StaticArrays/atiPw/src/mapreduce.jl:259 [inlined]
    [8] count
      @ ~/.julia/packages/StaticArrays/atiPw/src/mapreduce.jl:259 [inlined]
    [9] check_unit_cell(unit_cell_matrix::SMatrix{3, 3, Quantity{Float64}, 9}, cutoff::Quantity{Float64, 𝐋, Unitful.FreeUnits{(nm,), 𝐋, nothing}}; printerr::Bool)
      @ CellListMap ~/.julia/packages/CellListMap/rqPY5/src/CellOperations.jl:547
   [10] check_unit_cell(unit_cell_matrix::SMatrix{3, 3, Quantity{Float64}, 9}, cutoff::Quantity{Float64, 𝐋, Unitful.FreeUnits{(nm,), 𝐋, nothing}})
      @ CellListMap ~/.julia/packages/CellListMap/rqPY5/src/CellOperations.jl:537
   [11] CellListMap.Box(unit_cell_matrix::SMatrix{3, 3, Quantity{Float64}, 9}, cutoff::Quantity{Float64, 𝐋, Unitful.FreeUnits{(nm,), 𝐋, nothing}}, lcell::Int64, #unused#::Type{CellListMap.OrthorhombicCell})
      @ CellListMap ~/.julia/packages/CellListMap/rqPY5/src/Box.jl:167
   [12] Box
      @ ~/.julia/packages/CellListMap/rqPY5/src/Box.jl:268 [inlined]
   [13] #Box#14
      @ ~/.julia/packages/CellListMap/rqPY5/src/Box.jl:270 [inlined]
   [14] CellListMap.Box(sides::SVector{3, Quantity{Float64, 𝐋, Unitful.FreeUnits{(nm,), 𝐋, nothing}}}, cutoff::Quantity{Float64, 𝐋, Unitful.FreeUnits{(nm,), 𝐋, nothing}})
      @ CellListMap ~/.julia/packages/CellListMap/rqPY5/src/Box.jl:270
   [15] CellListMapNeighborFinder(; nb_matrix::BitMatrix, matrix_14::BitMatrix, n_steps::Int64, x0::Nothing, unit_cell::Nothing, number_of_batches::Tuple{Int64, Int64}, dist_cutoff::Quantity{Float64, 𝐋, Unitful.FreeUnits{(nm,), 𝐋, nothing}})
      @ Molly ~/projects/CESMIX/Molly.jl/src/neighbors.jl:316
   [16] macro expansion
      @ ~/projects/CESMIX/Molly.jl/test/basic.jl:119 [inlined]
   [17] macro expansion
      @ ~/builds/julia-1.7.1/share/julia/stdlib/v1.7/Test/src/Test.jl:1283 [inlined]
   [18] top-level scope
      @ ~/projects/CESMIX/Molly.jl/test/basic.jl:118
   [19] include(fname::String)
      @ Base.MainInclude ./client.jl:451
   [20] top-level scope
      @ ~/projects/CESMIX/Molly.jl/test/runtests.jl:70
   [21] include(fname::String)
      @ Base.MainInclude ./client.jl:451
   [22] top-level scope
      @ none:6
   [23] eval
      @ ./boot.jl:373 [inlined]
   [24] exec_options(opts::Base.JLOptions)
      @ Base ./client.jl:268
   [25] _start()
      @ Base ./client.jl:495
Test Summary:  | Pass  Error  Total
Neighbor lists |    4      1      5
ERROR: LoadError: Some tests did not pass: 4 passed, 0 failed, 1 errored, 0 broken.
in expression starting at /home/leios/projects/CESMIX/Molly.jl/test/basic.jl:117
in expression starting at /home/leios/projects/CESMIX/Molly.jl/test/runtests.jl:61
ERROR: Package Molly errored during testing

It seems like the zero function in Unitful will purposefully throw an error to a Quantity Type without a unit?

zero(x::AbstractQuantity) = Quantity(zero(x.val), unit(x))
zero(x::AffineQuantity) = Quantity(zero(x.val), absoluteunit(x))
zero(x::Type{<:AbstractQuantity{T}}) where {T} = throw(ArgumentError("zero($x) not defined."))
zero(x::Type{<:AbstractQuantity{T,D}}) where {T,D} = zero(T) * upreferred(D)
zero(x::Type{<:AbstractQuantity{T,D,U}}) where {T,D,U<:ScalarUnits} = zero(T)*U()
zero(x::Type{<:AbstractQuantity{T,D,U}}) where {T,D,U<:AffineUnits} = zero(T)*absoluteunit(U())

The zero function is used in the following block for CellListMaps:

function check_unit_cell(unit_cell_matrix::SMatrix{3},cutoff;printerr=true)
    ...
    if count(el -> el < zero(eltype(unit_cell_matrix)), unit_cell_matrix) != 0
         printerr && println("UNIT CELL CHECK FAILED: unit cell matrix components be strictly positive.")
         check = false
    end
    ...
end

I've gotten this error on two different machines, but I might still be doing something wrong locally. Here is my ] st:

     Project Molly v0.13.0
      Status `~/projects/CESMIX/Molly.jl/Project.toml`
  [a963bdd2] AtomsBase v0.2.3
  [de9282ab] BioStructures v1.2.1
  [052768ef] CUDA v3.12.0
  [69e1c6dd] CellListMap v0.8.3
  [d360d2e6] ChainRulesCore v1.15.5
  [46823bd8] Chemfiles v0.10.3
  [5ae59095] Colors v0.12.8
  [861a8166] Combinatorics v1.0.2
  [864edb3b] DataStructures v0.18.13
  [b4f34e82] Distances v0.10.7
  [31c24e10] Distributions v0.25.73
  [8f5d6c58] EzXML v1.1.0
  [cc61a311] FLoops v0.2.0
  [f6369f11] ForwardDiff v0.10.32
  [5ab0869b] KernelDensity v0.6.5
  [b8a86587] NearestNeighbors v0.4.11
  [189a3867] Reexport v1.2.2
  [ae029012] Requires v1.3.0
  [90137ffa] StaticArrays v1.5.7
  [1986cc42] Unitful v1.12.0
  [f31437dd] UnitfulChainRules v0.1.2
  [e88e6eb3] Zygote v0.6.44
  [37e2e46d] LinearAlgebra
  [9a3f8284] Random
  [2f01184e] SparseArrays
  [10745b16] Statistics

note that the tests do pass on CellListMaps@v0.7.21

jgreener64 commented 2 years ago

Yes this affects CI as well. It looks like a minimum example on CellListMap 0.8.3 and Julia 1.8.1 is

using CellListMap, StaticArrays, Unitful
CellListMap.Box(SVector(2.0, 2.0, 2.0)u"nm", 1.0u"nm")
ERROR: ArgumentError: zero(Quantity{Float64}) not defined.
Stacktrace:
  [1] zero(x::Type{Quantity{Float64}})
    @ Unitful ~/.julia/packages/Unitful/SUQzL/src/quantities.jl:386
  [2] (::CellListMap.var"#95#96"{SMatrix{3, 3, Quantity{Float64}, 9}})(el::Quantity{Float64, 𝐋, Unitful.FreeUnits{(nm,), 𝐋, nothing}})
    @ CellListMap ~/.julia/packages/CellListMap/rqPY5/src/CellOperations.jl:547
  [3] (::StaticArrays.var"#194#195"{CellListMap.var"#95#96"{SMatrix{3, 3, Quantity{Float64}, 9}}})(x::Quantity{Float64, 𝐋, Unitful.FreeUnits{(nm,), 𝐋, nothing}})
    @ StaticArrays ~/.julia/packages/StaticArrays/atiPw/src/mapreduce.jl:259
  [4] macro expansion
    @ ~/.julia/packages/StaticArrays/atiPw/src/mapreduce.jl:140 [inlined]
  [5] _mapfoldl
    @ ~/.julia/packages/StaticArrays/atiPw/src/mapreduce.jl:115 [inlined]
  [6] _mapreduce
    @ ~/.julia/packages/StaticArrays/atiPw/src/mapreduce.jl:113 [inlined]
  [7] #count#193
    @ ~/.julia/packages/StaticArrays/atiPw/src/mapreduce.jl:259 [inlined]
  [8] count
    @ ~/.julia/packages/StaticArrays/atiPw/src/mapreduce.jl:259 [inlined]
  [9] check_unit_cell(unit_cell_matrix::SMatrix{3, 3, Quantity{Float64}, 9}, cutoff::Quantity{Float64, 𝐋, Unitful.FreeUnits{(nm,), 𝐋, nothing}}; printerr::Bool)
    @ CellListMap ~/.julia/packages/CellListMap/rqPY5/src/CellOperations.jl:547
 [10] check_unit_cell(unit_cell_matrix::SMatrix{3, 3, Quantity{Float64}, 9}, cutoff::Quantity{Float64, 𝐋, Unitful.FreeUnits{(nm,), 𝐋, nothing}})
    @ CellListMap ~/.julia/packages/CellListMap/rqPY5/src/CellOperations.jl:536
 [11] Box(unit_cell_matrix::SMatrix{3, 3, Quantity{Float64}, 9}, cutoff::Quantity{Float64, 𝐋, Unitful.FreeUnits{(nm,), 𝐋, nothing}}, lcell::Int64, #unused#::Type{OrthorhombicCell})
    @ CellListMap ~/.julia/packages/CellListMap/rqPY5/src/Box.jl:167
 [12] Box
    @ ~/.julia/packages/CellListMap/rqPY5/src/Box.jl:268 [inlined]
 [13] #Box#14
    @ ~/.julia/packages/CellListMap/rqPY5/src/Box.jl:270 [inlined]
 [14] Box(sides::SVector{3, Quantity{Float64, 𝐋, Unitful.FreeUnits{(nm,), 𝐋, nothing}}}, cutoff::Quantity{Float64, 𝐋, Unitful.FreeUnits{(nm,), 𝐋, nothing}})
    @ CellListMap ~/.julia/packages/CellListMap/rqPY5/src/Box.jl:270
 [15] top-level scope
    @ REPL[9]:1

@lmiq should I open an issue on CellListMap for this?

lmiq commented 2 years ago

No need for it. I will release just right now a patch in CellListMap v0.8.4

(I added a workaround for the problem, but I think the bug is on LinearAlgebra: diagm https://github.com/JuliaLang/julia/issues/46829)

(this occurs in CellListMap v0.8.2 and v0.8.3 only).

lmiq commented 2 years ago

Should be fixed now (v0.8.4 is released).

leios commented 2 years ago

It seems like the tests now pass locally. Thanks for the quick response here!