Move JSON3 dependency into extension module (#907)

.github/workflows/ci.yaml

@@ -14,79 +14,46 @@ concurrency:
   cancel-in-progress: ${{ startsWith(github.ref, 'refs/pull/') }}
 
 jobs:
-  #
-  # Unit tests
-  #
   test:
-    name: Julia stable - ${{ matrix.os }} - ${{ matrix.payload }}
+    name: Julia ${{ matrix.mode }} - ${{ matrix.os }} - ${{ matrix.payload }}
     runs-on: ${{ matrix.os }}
     strategy:
       fail-fast: false
       matrix:
         include:
-          - {os: ubuntu-latest,  payload: "fast,example" }
-          - {os: macOS-latest,   payload: fast           }
-          - {os: windows-latest, payload: fast           }
-          - {os: ubuntu-latest,  payload: mpi            }
+          - {mode: 'stable',  os: ubuntu-latest,  payload: "fast,example" }
+          - {mode: 'stable',  os: macOS-latest,   payload: fast           }
+          - {mode: 'stable',  os: windows-latest, payload: fast           }
+          - {mode: 'stable',  os: ubuntu-latest,  payload: "fast,mpi"     }
+          - {mode: 'nightly', os: ubuntu-latest,  payload: fast           }
     env:
       GKS_ENCODING: "utf8"
-      GKSwstype: "100"    # Needed for Plots-related tests
-      PLOTS_TEST: "true"  # Needed for Plots-related tests
+      GKSwstype: "100"       # Needed for Plots-related tests
+      PLOTS_TEST: "true"     # Needed for Plots-related tests
+      DFTK_TEST_NPROCS: "2"  # For MPI parallelised tests
 
     steps:
       - uses: actions/checkout@v4
-      - uses: julia-actions/setup-julia@v1
-        with:
-          version: '1.8'
-          arch: x64
-      - uses: julia-actions/cache@v1
-      - uses: julia-actions/julia-buildpkg@v1
-
-      - uses: julia-actions/julia-runtest@v1
-        if: ${{ matrix.payload != 'mpi' }}
-        env:
-          DFTK_TEST_ARGS: ${{ matrix.payload }}
-      - name: Execute MPI-parallel tests
-        run: |
-          julia --project -e '
-            using Pkg; Pkg.build(); Pkg.precompile()
-            Pkg.add("MPI"); using MPI; MPI.install_mpiexecjl()
-            Pkg.test(; test_args=["minimal"])
-          '
-          $HOME/.julia/bin/mpiexecjl -np 2 julia --check-bounds=yes --depwarn=yes --project --color=yes -e 'using Pkg; Pkg.test(coverage=true)'
-        if: ${{ matrix.payload == 'mpi' }}
-        env:
-          DFTK_TEST_ARGS: fast
 
-      - uses: julia-actions/julia-processcoverage@v1
-      - uses: codecov/codecov-action@v3
+      - name: "Setup Julia stable"
+        uses: julia-actions/setup-julia@v1
         with:
-          files: lcov.info
-  #
-  # Nightly
-  #
-  nightly:
-    name: Julia nightly - ubuntu-latest
-    runs-on: ubuntu-latest
-    strategy:
-      fail-fast: false
-    env:
-      GKS_ENCODING: "utf8"
-      GKSwstype: "100"    # Needed for Plots-related tests
-      PLOTS_TEST: "true"  # Needed for Plots-related tests
-
-    steps:
-      - uses: actions/checkout@v4
-      - uses: julia-actions/setup-julia@v1
+          version: '1.9'
+          arch: x64
+        if: ${{ matrix.mode == 'stable' }}
+      - name: "Setup Julia nightly"
+        uses: julia-actions/setup-julia@v1
         with:
-          version: nightly
+          version: 'nightly'
           arch: x64
+        if: ${{ matrix.mode == 'nightly' }}
+
       - uses: julia-actions/cache@v1
       - uses: julia-actions/julia-buildpkg@v1
       - uses: julia-actions/julia-runtest@v1
         env:
-          DFTK_TEST_ARGS: fast
+          DFTK_TEST_ARGS: ${{ matrix.payload }}
       - uses: julia-actions/julia-processcoverage@v1
       - uses: codecov/codecov-action@v3
         with:
-          file: lcov.info
+          files: lcov.info

Project.toml

@@ -49,6 +49,14 @@ TimerOutputs = "a759f4b9-e2f1-59dc-863e-4aeb61b1ea8f"
 Unitful = "1986cc42-f94f-5a68-af5c-568840ba703d"
 UnitfulAtomic = "a7773ee8-282e-5fa2-be4e-bd808c38a91a"
 
+[weakdeps]
+JSON3 = "0f8b85d8-7281-11e9-16c2-39a750bddbf1"
+WriteVTK = "64499a7a-5c06-52f2-abe2-ccb03c286192"
+
+[extensions]
+DFTKJSON3Ext = "JSON3"
+DFTKWriteVTK = "WriteVTK"
+
 [compat]
 AbstractFFTs = "1"
 Artifacts = "1"
@@ -65,8 +73,9 @@ Interpolations = "0.14, 0.15"
 IntervalArithmetic = "0.20"
 IterTools = "1"
 IterativeSolvers = "0.9"
+JSON3 = "1"
 LazyArtifacts = "1.3"
-Libxc = "0.3.14"
+Libxc = "0.3.17"
 LineSearches = "7"
 LinearAlgebra = "1"
 LinearMaps = "3"
@@ -95,7 +104,8 @@ Statistics = "1"
 TimerOutputs = "0.5.12"
 Unitful = "1"
 UnitfulAtomic = "1"
-julia = "1.8"
+WriteVTK = "1"
+julia = "1.9"
 
 [extras]
 ASEconvert = "3da9722f-58c2-4165-81be-b4d7253e8fd2"

README.md

@@ -46,7 +46,7 @@ For getting started with DFTK, see [our documentation](https://docs.dftk.org):
 - [Tutorial](https://docs.dftk.org/stable/guide/tutorial/)
 - [Basic DFT examples](https://docs.dftk.org/stable/examples/metallic_systems/)
 
-Note that at least **Julia 1.8** is required.
+Note that at least **Julia 1.9** is required.
 
 ## Support and citation
 DFTK is mostly developed as part of academic research.

src/external/json3.jl → ext/DFTKJSON3Ext.jl

@@ -1,4 +1,9 @@
-function save_scfres_master(filename::AbstractString, scfres::NamedTuple, ::Val{:json})
+module DFTKJSON3Ext
+using JSON3
+using DFTK
+using DFTK: todict
+
+function DFTK.save_scfres_master(filename::AbstractString, scfres::NamedTuple, ::Val{:json})
     # TODO Quick and dirty solution for now.
     #      The better approach is to integrate with StructTypes.jl
 
@@ -14,3 +19,5 @@ function save_scfres_master(filename::AbstractString, scfres::NamedTuple, ::Val{
 end
 
 #TODO introduce `todict` functions for all sorts of datastructures (basis, ...)
+
+end

src/external/vtkio.jl → ext/DFTKWriteVTK.jl

@@ -1,5 +1,9 @@
-function save_scfres_master(filename::AbstractString, scfres::NamedTuple, ::Val{:vts};
-                            save_ψ=false, extra_data=Dict{String,Any}())
+module DFTKWriteVTK
+using WriteVTK
+using DFTK
+
+function DFTK.save_scfres_master(filename::AbstractString, scfres::NamedTuple, ::Val{:vts};
+                                 save_ψ=false, extra_data=Dict{String,Any}())
     !mpi_master() && error(
         "This function should only be called on MPI master after the k-point data has " *
         "been gathered with `gather_kpts`."
@@ -50,3 +54,5 @@ function save_scfres_master(filename::AbstractString, scfres::NamedTuple, ::Val{
 
     only(WriteVTK.vtk_save(vtkfile))
 end
+
+end

src/DFTK.jl

@@ -220,6 +220,8 @@ include("workarounds/gpu_arrays.jl")
 
 
 function __init__()
+    # TODO No idea how to get rid of these requires below:
+
     # Use "@require" to only include fft_generic.jl once IntervalArithmetic or
     # DoubleFloats has been loaded (via a "using" or an "import").
     # See https://github.com/JuliaPackaging/Requires.jl for details.
@@ -233,13 +235,15 @@ function __init__()
     @require DoubleFloats="497a8b3b-efae-58df-a0af-a86822472b78" begin
         !isdefined(DFTK, :GENERIC_FFT_LOADED) && include("workarounds/fft_generic.jl")
     end
-    @require Plots="91a5bcdd-55d7-5caf-9e0b-520d859cae80"    include("plotting.jl")
+
+    # TODO Keep these requires for now as there are open PRs changing these files.
     @require JLD2="033835bb-8acc-5ee8-8aae-3f567f8a3819"     include("external/jld2io.jl")
-    @require JSON3="0f8b85d8-7281-11e9-16c2-39a750bddbf1"    include("external/json3.jl")
-    @require WriteVTK="64499a7a-5c06-52f2-abe2-ccb03c286192" include("external/vtkio.jl")
+    @require Plots="91a5bcdd-55d7-5caf-9e0b-520d859cae80"    include("plotting.jl")
     @require wannier90_jll="c5400fa0-8d08-52c2-913f-1e3f656c1ce9" begin
         include("external/wannier90.jl")
     end
+
+    # TODO Move out into extension module
     @require CUDA="052768ef-5323-5732-b1bb-66c8b64840ba"  begin
         include("workarounds/cuda_arrays.jl")
     end

src/SymOp.jl

@@ -18,9 +18,9 @@
 #   (Uu)(G) = e^{i G τ} conj(u(-S^-1 G))
 
 # Tolerance to consider two atomic positions as equal (in relative coordinates).
-const SYMMETRY_TOLERANCE = 1e-5
+const SYMMETRY_TOLERANCE = convert(Float64, @load_preference("symmetry_tolerance", 1e-5))
 
-is_approx_integer(r; tol=SYMMETRY_TOLERANCE) = all(ri -> abs(ri - round(ri)) ≤ tol, r)
+is_approx_integer(r; atol=SYMMETRY_TOLERANCE) = all(ri -> abs(ri - round(ri)) ≤ atol, r)
 
 struct SymOp{T <: Real}
     # (Uu)(x) = u(W x + w) in real space
@@ -44,7 +44,7 @@ end
 
 Base.:(==)(op1::SymOp, op2::SymOp) = op1.W == op2.W && op1.w == op2.w
 function Base.isapprox(op1::SymOp, op2::SymOp; atol=SYMMETRY_TOLERANCE)
-    op1.W == op2.W && is_approx_integer(op1.w - op2.w; tol=atol)
+    op1.W == op2.W && is_approx_integer(op1.w - op2.w; atol)
 end
 Base.one(::Type{SymOp}) = one(SymOp{Bool})  # Not sure about this method
 Base.one(::Type{SymOp{T}}) where {T} = SymOp(Mat3{Int}(I), Vec3(zeros(T, 3)))

src/common/timer.jl

@@ -21,4 +21,5 @@ end
 
 function set_timer_enabled!(state=true)
     @set_preferences!("timer_enabled" => string(state))
+    @info "timer_enabled preference changed. Restart julia to see the effect."
 end

src/external/atomsbase.jl

@@ -89,6 +89,9 @@ function AtomsBase.atomic_system(lattice::AbstractMatrix{<:Number},
         end
         if element isa ElementPsp
             kwargs[:pseudopotential] = element.psp.identifier
+            if element.psp isa PspUpf
+                kwargs[:pseudopotential_kwargs] = (; element.psp.rcut)
+            end
         elseif element isa ElementCoulomb
             kwargs[:pseudopotential] = ""
         elseif !(element isa ElementCoulomb)

src/external/jld2io.jl

@@ -1,25 +1,3 @@
-function ScfSaveCheckpoints(filename="dftk_scf_checkpoint.jld2"; keep=false, overwrite=false)
-    # TODO Save only every 30 minutes or so
-    function callback(info)
-        if info.n_iter == 1
-            isfile(filename) && !overwrite && error(
-                "Checkpoint $filename exists. Use 'overwrite=true' to force overwriting."
-            )
-        end
-        if info.stage == :finalize
-            if mpi_master() && !keep
-                isfile(filename) && rm(filename)  # Cleanup checkpoint
-            end
-        else
-            scfres = (; (k => v for (k, v) in pairs(info) if !startswith(string(k), "ρ"))...)
-            scfres = merge(scfres, (; ρ=info.ρout))
-            save_scfres(filename, scfres)
-        end
-        info
-    end
-end
-
-
 function save_scfres_master(file::AbstractString, scfres::NamedTuple, ::Val{:jld2})
     !mpi_master() && error(
         "This function should only be called on MPI master after the k-point data has " *

src/scf/scf_callbacks.jl

@@ -2,7 +2,28 @@
 Adds simplistic checkpointing to a DFTK self-consistent field calculation.
 Requires JLD2 to be loaded.
 """
-function ScfSaveCheckpoints end  # implementation in src/jld2io.jl
+function ScfSaveCheckpoints(filename="dftk_scf_checkpoint.jld2"; keep=false, overwrite=false)
+    # TODO Save only every 30 minutes or so
+    function callback(info)
+        if info.n_iter == 1
+            isfile(filename) && !overwrite && error(
+                "Checkpoint $filename exists. Use 'overwrite=true' to force overwriting."
+            )
+        end
+        if info.stage == :finalize
+            if mpi_master() && !keep
+                isfile(filename) && rm(filename)  # Cleanup checkpoint
+            end
+        else
+            scfres = (; (k => v for (k, v) in pairs(info) if !startswith(string(k), "ρ"))...)
+            scfres = merge(scfres, (; ρ=info.ρout))
+            save_scfres(filename, scfres)
+        end
+        info
+    end
+end
+
+
 
 """
 Plot the trace of an SCF, i.e. the absolute error of the total energy at
@@ -155,5 +176,3 @@ function ScfDiagtol(; ratio_ρdiff=0.2, diagtol_min=nothing, diagtol_max=0.03)
         diagtol
     end
 end
-
-

src/symmetry.jl

@@ -162,7 +162,7 @@ function _check_symmetries(symmetries::AbstractVector{<:SymOp}, lattice, atom_gr
             for coord in group_positions
                 # If all elements of a difference in diffs is integer, then
                 # W * coord + w and pos are equivalent lattice positions
-                if !any(c -> is_approx_integer(W * coord + w - c; tol=tol_symmetry), group_positions)
+                if !any(c -> is_approx_integer(W * coord + w - c; atol=tol_symmetry), group_positions)
                     error("Issue in symmetry determination: Cannot map the atom at position " *
                           "$coord to another atom of the same element under the symmetry " *
                           "operation (W, w):\n($W, $w)")

test/aqua.jl

@@ -0,0 +1,12 @@
+@testitem "Aqua" tags=[:dont_test_mpi] begin
+    # TODO For now disable type piracy check, as we use that at places to patch
+    #      up missing functionality. Should disable this on a more fine-grained scale.
+
+    using DFTK
+    using Aqua
+    Aqua.test_all(DFTK;
+                  ambiguities=false,
+                  piracies=false,
+                  deps_compat=(check_extras=false, ),
+                  stale_deps=(ignore=[:Primes, ], ))
+end

test/runexamples.jl → test/examples.jl

@@ -1,4 +1,4 @@
-@testitem "Run examples" tags=[:example] begin
+@testitem "Run examples" tags=[:slow, :example] begin
     function list_examples()
         res = String[]
         for (root, dirs, files) in walkdir(joinpath(@__DIR__, "..", "examples"))

test/external/atomsbase.jl

@@ -24,8 +24,8 @@
     @test system[:, :magnetic_moment] == magnetic_moments
 
     parsed = DFTK.parse_system(system)
-    @test parsed.lattice   ≈ lattice   atol=1e-13
-    @test parsed.positions ≈ positions atol=1e-13
+    @test parsed.lattice   ≈ lattice   atol=5e-13
+    @test parsed.positions ≈ positions atol=5e-13
     for i in 1:4
         @test iszero(parsed.magnetic_moments[i][1:2])
         @test parsed.magnetic_moments[i][3] == magnetic_moments[i]
@@ -116,8 +116,8 @@ end
     system      = periodic_system(atoms, lattice; fractional=true)
 
     let model = Model(system)
-        @test model.lattice   ≈ pos_lattice atol=1e-13
-        @test model.positions ≈ pos_units   atol=1e-13
+        @test model.lattice   ≈ pos_lattice atol=5e-13
+        @test model.positions ≈ pos_units   atol=5e-13
         @test model.spin_polarization == :none
 
         @test length(model.atoms) == 4
@@ -137,8 +137,8 @@ end
         @test system[4, :pseudopotential] == "hgh/pbe/c-q4.hgh"
 
         parsed = DFTK.parse_system(system)
-        @test parsed.lattice   ≈ pos_lattice atol=1e-13
-        @test parsed.positions ≈ pos_units   atol=1e-13
+        @test parsed.lattice   ≈ pos_lattice atol=5e-13
+        @test parsed.positions ≈ pos_units   atol=5e-13
         @test isempty(parsed.magnetic_moments)
 
         @test length(parsed.atoms) == 4
@@ -157,8 +157,8 @@ end
         @test system[4, :pseudopotential] == "hgh/lda/c-q4.hgh"
 
         model = Model(system)
-        @test model.lattice   ≈ pos_lattice atol=1e-13
-        @test model.positions ≈ pos_units   atol=1e-13
+        @test model.lattice   ≈ pos_lattice atol=5e-13
+        @test model.positions ≈ pos_units   atol=5e-13
         @test model.spin_polarization == :none
 
         @test length(model.atoms) == 4