Merge pull request #55 from SebastianM-C/claude/multi-threading-review-9hat4i

Fix laser equality/hashing and actually run the thread-safety test

Author: SebastianM-C

.github/workflows/ci.yml

@@ -9,7 +9,7 @@ on:
 
 jobs:
   test:
-    name: Julia ${{ matrix.julia-version }} - ${{ matrix.os }} - ${{ matrix.julia-arch }} - threads ${{ matrix.julia-threads }}
+    name: Julia ${{ matrix.version }} - ${{ matrix.os }} - ${{ matrix.julia-arch }} - threads ${{ matrix.julia-threads }}
     runs-on: ${{ matrix.os }}
     env:
       JULIA_NUM_THREADS: ${{ matrix.julia-threads }}

Project.toml

@@ -1,6 +1,6 @@
 name = "LaserTypes"
 uuid = "e07c0bfa-524c-4f35-a151-c3dd916fa2f0"
-version = "0.2.1"
+version = "0.2.2"
 authors = ["Sebastian Micluța-Câmpeanu <m.c.sebastian95@gmail.com>", "Petru-Vlad Toma <tomapv@gmail.com>"]
 
 [deps]
@@ -27,7 +27,7 @@ TaskLocalValues = "0.1.3"
 UnPack = "1.0"
 Unitful = "1.0"
 UnitfulAtomic = "0.3, 1.0"
-julia = "1"
+julia = "1.10"
 
 [extras]
 CSV = "336ed68f-0bac-5ca0-87d4-7b16caf5d00b"

src/utils.jl

@@ -23,40 +23,32 @@ end
 @inline inv_rotate(R, r) = R \ r
 @inline inv_rotate(::UniformScaling, r) = r
 
+# The cache field is excluded from equality and hashing: it holds transient
+# task-local scratch state, so including it would make the results depend on
+# the evaluation history of the current task.
 function Base.:(==)(a::AbstractLaser, b::AbstractLaser)
-    typeof(a) == typeof(b) &&
-    fundamental_constants(a) == fundamental_constants(b) &&
-    immutable_cache(a) == immutable_cache(b) &&
-    mutable_cache(a) == mutable_cache(b) &&
-    geometry(a) == geometry(b) &&
-    polarization(a) == polarization(b) &&
-    profile(a) == profile(b) &&
-    true
+    typeof(a) == typeof(b) || return false
+    for f in fieldnames(typeof(a))
+        f === :cache && continue
+        getfield(a, f) == getfield(b, f) || return false
+    end
+    return true
 end
 
 function Base.isequal(a::AbstractLaser, b::AbstractLaser)
-    t1 = typeof(a)
-    t2 = typeof(b)
-
-    isequal(t1, t2) &&
-    fundamental_constants(a) == fundamental_constants(b) &&
-    immutable_cache(a) == immutable_cache(b) &&
-    mutable_cache(a) == mutable_cache(b) &&
-    geometry(a) == mutable_cache(b) &&
-    polarization(a) == polarization(b) &&
-    profile(a) == profile(b) &&
-    true
+    typeof(a) == typeof(b) || return false
+    for f in fieldnames(typeof(a))
+        f === :cache && continue
+        isequal(getfield(a, f), getfield(b, f)) || return false
+    end
+    return true
 end
 
 function Base.hash(l::AbstractLaser, h::UInt)
-    constants = fundamental_constants(l)
-    derived = immutable_cache(l)
-    cache = mutable_cache(l)
-    geo = geometry(l)
-    pol = polarization(l)
-    prof = profile(l)
-
-    typename = Symbol(typeof(l))
-    hash(prof, hash(pol, hash(geo, hash(cache, hash(derived, hash(constants,
-        hash(typename, h)))))))
+    h = hash(Symbol(typeof(l)), h)
+    for f in fieldnames(typeof(l))
+        f === :cache && continue
+        h = hash(getfield(l, f), h)
+    end
+    return h
 end

test/runtests.jl

@@ -8,6 +8,14 @@ using Test, SafeTestsets
     @safetestset "Laguerre-Gauss" begin include("Laguerre-Gauss/laguerre-gauss.jl") end
     @safetestset "Fμν" begin include("faraday.jl") end
     @safetestset "Derived" begin include("derived.jl") end
+    @safetestset "Threads" begin include("threads.jl") end
     @safetestset "Show" begin include("show.jl") end
-    @safetestset "QA" begin include("qa.jl") end
+    # JET reports upstream runtime dispatch on Julia pre-releases (e.g. Base
+    # printing internals and StaticArrays construction on 1.13-DEV), so only
+    # run the QA tests on release versions.
+    if isempty(VERSION.prerelease)
+        @safetestset "QA" begin include("qa.jl") end
+    else
+        @info "Skipping JET QA tests on pre-release Julia" VERSION
+    end
 end