Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Seniority number in jj-coupling #64

Merged
merged 9 commits into from
Jun 13, 2020
Merged

Seniority number in jj-coupling #64

merged 9 commits into from
Jun 13, 2020

Conversation

jagot
Copy link
Member

@jagot jagot commented Jun 11, 2020


@jagot jagot requested a review from mortenpi June 11, 2020 19:57
@jagot
Copy link
Member Author

jagot commented Jun 11, 2020

The CSF tests based on references generated by GRASP are broken since the CLSs don't contain seniority numbers, which are now required for IntermediateTerm also for jj-coupling. Don't know how we should solve this yet.

@mortenpi
Copy link
Member

mortenpi commented Jun 11, 2020

GRASP does support seniority IIRC and prints that to the CSL (if needed). But I think all the cases we have in the tests are "non-degenerate", so there is no need for seniority. So the tests should work without issues, but the parser probably needs to be updated.

src/intermediate_terms.jl Outdated Show resolved Hide resolved
src/intermediate_terms.jl Show resolved Hide resolved
src/csfs.jl Show resolved Hide resolved
test/jj_terms.jl Outdated Show resolved Hide resolved
Manifest.toml Show resolved Hide resolved
src/csfs.jl Outdated Show resolved Hide resolved
src/csfs.jl Outdated
Comment on lines 50 to 51
CSF(config, subshell_terms, terms::Vector{<:Real}) =
CSF(config, subshell_terms, convert.(HalfInt, terms))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes stack overflows, e.g. with

julia> CSF(rc"1s2 2p- 2p", HalfInt.([0, 1//2, 3//2]), HalfInt.([0, 1//2, 1]))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know if it applies anymore, since the subshell terms need to be IntermediateTerm.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does, since the constructor is still there. You're right in that the way I call it is not really valid, but it should fail with a missing method error then, not cause a stack overflow.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I cannot reproduce anymore:

julia> CSF(rc"1s 2s", [IntermediateTerm(half(1), Seniority(1)), IntermediateTerm(half(1), Seniority(1))], HalfInt.([1/2, 1]))
1s   2s
  1/2  1/2
   1/2    1+

julia> CSF(rc"1s 2s", [IntermediateTerm(1/2, Seniority(1)), IntermediateTerm(1/2, Seniority(1))], [1/2, 1])
1s   2s
  1/2  1/2
   1/2    1+

Copy link
Member

@mortenpi mortenpi Jun 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's because the second argument is IntermediateTerms. It crashes with

julia> CSF(rc"1s2 2p- 2p", HalfInt.([0, 1//2, 3//2]), HalfInt.([0, 1//2, 1]))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4d8981a:

julia> CSF(rc"1s 2s", [IntermediateTerm(half(1), Seniority(1)), IntermediateTerm(half(1), Seniority(1))], HalfInt.([1/2, 1]))
1s   2s
  1/2  1/2
   1/2    1+

julia> CSF(rc"1s 2s", [IntermediateTerm(1/2, Seniority(1)), IntermediateTerm(1/2, Seniority(1))], [1/2, 1])
1s   2s
  1/2  1/2
   1/2    1+

julia> CSF(rc"1s2 2p- 2p", HalfInt.([0, 1//2, 3//2]), HalfInt.([0, 1//2, 1]))
ERROR: MethodError: no method matching CSF(::Configuration{RelativisticOrbital{Int64}}, ::Array{Half{Int64},1}, ::Array{Half{Int64},1})
Closest candidates are:
  CSF(::Configuration{O}, ::Array{IntermediateTerm{T,S},1}, ::Array{T,1}) where {O<:(Union{#s41, #s40} where #s40<:RelativisticOrbital where #s41<:Orbital), T<:Union{Term, Half{Int64}}, S} at /Users/jagot/.julia/dev/AtomicLevels/src/csfs.jl:10
  CSF(::Any, ::Array{#s19,1} where #s19<:IntermediateTerm, ::Array{#s18,1} where #s18<:Real) at /Users/jagot/.julia/dev/AtomicLevels/src/csfs.jl:24
Stacktrace:
 [1] top-level scope at REPL[5]:1
 [2] eval(::Module, ::Any) at ./boot.jl:331
 [3] eval_user_input(::Any, ::REPL.REPLBackend) at /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.4/REPL/src/REPL.jl:86
 [4] run_backend(::REPL.REPLBackend) at /Users/jagot/.julia/packages/Revise/tV8FE/src/Revise.jl:1165
 [5] top-level scope at REPL[1]:0

src/intermediate_terms.jl Outdated Show resolved Hide resolved
src/intermediate_terms.jl Outdated Show resolved Hide resolved
src/jj_terms.jl Outdated Show resolved Hide resolved
Copy link
Member

@mortenpi mortenpi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Some additional documentation would always be nice, but we can potentially do that in #61 (which we should look into merging anyway 😛).

@jagot
Copy link
Member Author

jagot commented Jun 13, 2020

Closes #15

@mortenpi
Copy link
Member

Closes #15

Also #51 and #59 I think.

@jagot jagot merged commit 820a639 into master Jun 13, 2020
@jagot jagot deleted the jj-seniority branch June 13, 2020 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants