Using higher correlation orders causes error

[GeorgeSimmons2]

With the latest version of ACEpotentials, creating a model using a correlation order of 4 seems to return an error. Similarly for correlation order above 4.

julia> using ACEpotentials
julia> ace1_model(elements = [:Si],
                               order = 4,
                               totaldegree = 16,
                               rcut = 6.0)

Output:

ERROR: InexactError: Float64(0.3903577709314921 + 0.0013278005676369202im)
Stacktrace:
 [1] Real
   @ ./complex.jl:44 [inlined]
 [2] convert
   @ ./number.jl:7 [inlined]
 [3] push!
   @ ./array.jl:1118 [inlined]
 [4] _rpi_A2B_matrix(cgen::RepLieGroups.O3.Rot3DCoeffs_real{0, ComplexF64}, spec::Vector{Vector{@NamedTuple{…}}})
   @ EquivariantModels ~/.julia/packages/EquivariantModels/MGMmH/src/builder.jl:135
 [5] _generate_ace_model(rbasis::ACEpotentials.Models.SplineRnlrzzBasis{…}, Ytype::Symbol, AA_spec::Vector{…}, Vref::ACEpotentials.Models.OneBody{…}, level::ACEpotentials.Models.TotalDegree, pair_basis::ACEpotentials.Models.SplineRnlrzzBasis{…})
   @ ACEpotentials.Models ~/.julia/packages/ACEpotentials/lI0MC/src/models/ace.jl:108
 [6] ace_model(rbasis::ACEpotentials.Models.SplineRnlrzzBasis{…}, Ytype::Symbol, AA_spec::Vector{…}, level::ACEpotentials.Models.TotalDegree, pair_basis::ACEpotentials.Models.SplineRnlrzzBasis{…}, Vref::ACEpotentials.Models.OneBody{…})
   @ ACEpotentials.Models ~/.julia/packages/ACEpotentials/lI0MC/src/models/ace.jl:155
 [7] ace_model(; elements::Vector{…}, order::Int64, Ytype::Symbol, ZBL::Bool, E0s::Nothing, rin0cuts::StaticArraysCore.SMatrix{…}, rbasis::ACEpotentials.Models.SplineRnlrzzBasis{…}, rbasis_type::Symbol, maxl::Int64, maxn::Int64, maxq_fact::Float64, maxq::Symbol, init_Wradial::Symbol, level::ACEpotentials.Models.TotalDegree, max_level::Vector{…}, init_WB::Symbol, pair_maxn::Nothing, pair_basis::ACEpotentials.Models.SplineRnlrzzBasis{…}, pair_learnable::Bool, pair_transform::Tuple{…}, init_Wpair::Symbol, rng::Random.TaskLocalRNG)
   @ ACEpotentials.Models ~/.julia/packages/ACEpotentials/lI0MC/src/models/ace_heuristics.jl:219
 [8] ace1_model(; kwargs::@Kwargs{elements::Vector{Symbol}, order::Int64, totaldegree::Int64, rcut::Float64})
   @ ACEpotentials.ACE1compat ~/.julia/packages/ACEpotentials/lI0MC/src/ace1_compat.jl:406
 [9] top-level scope
   @ REPL[5]:1
Some type information was truncated. Use `show(err)` to see complete types.

[cortner]

Thanks for reporting. This is obviously bad. @zhanglw0521 is there a quick fix for this?

[zhanglw0521]

The problem lies in here - when using real spherical harmonics, as is the case in ACEpotentials.jl, we don't necessarily expect real coupling coefficients, and hence the reported error.

The fastest fix is to change Float64 in the line I pointed out above, to ComplexF64 (this can even be done locally, though it can be a bit risky...), and then the ace1_model can be built for arbitrarily large correlation order / polynomial degree. The drawback of this is that it will lead to complex couplings, which may be troublesome at the next stages where differentiation may be needed etc.

Another fix would be to use RepLieGroups.jl v0.1.0 in EquivariantModels.jl instead of v0.0.x, in which all the coupling coeffs are set to be real.

In any case, this should be something to change in EquivariantModels.jl, but would just be a minor patch. And the routine would be:

Update EquivariantModels.jl -> Update ACEpotentials.jl so to use the latest EM.jl -> Issue resolved.

Should I create an issue in EM to start the process?

[cortner]

we don't necessarily expect real coupling coefficients

I don't understand that. The AA basis is real, the BB basis is real, so why can't we use real coupling coeffs? From a representation theory perspective, the D matrices for real sh must be real therefore their tensor product must be real too. What am I missing?

[cortner]

Should I create an issue in EM to start the process?

Maybe an issue in both EM and ET with a failing test would be great.

I don't really want us to waste time on fixing EM to be honest, unless it's a small fix. I prefer that we focus on refactoring ACEpotentials. But if this issue still exists in ET then it should be fixed there for sure.

[zhanglw0521]

Surely we can keep it real, which is already the case in the latest RepLieGroups.jl (and hence it is also real in ET). It was just that we didn't enforce this in the old version of RepLieGroups.jl.

[zhanglw0521]

Perhaps I can put it this way: if our plan is to use ET in the future development of ACEpotentials.jl, then this won't be an issue as it is already fixed in ET. On the other hand, if we were to fix EM to fix this issue (which gives a relatively faster fix to this issue), it would indeed be a super small fix.

[cortner]

If it is small and you have the time, we'd be grateful. Let us know when it's fixed and I can then fix the version bounds.

[zhanglw0521]

Ok, great. I will file an issue under EQM and we can continue there.