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

jj2lsj's parity limitation #17

Open
jongrumer opened this issue May 20, 2019 · 9 comments · May be fixed by #45
Open

jj2lsj's parity limitation #17

jongrumer opened this issue May 20, 2019 · 9 comments · May be fixed by #45
Assignees
Labels
enhancement New feature or request

Comments

@jongrumer
Copy link
Member

As highlighted in a recent email by @cffischer, jj2lsj crashes on lists with both parities present, and does not print out any warnings ( I think?). Only @gaigalas knows how hard it would be to extend the routine, and how much time it would take. We all have many things to do.

I would consider this "enhancement" having relatively low priority, but should nevertheless be adressed at some point.

Maybe a quick solution would be to add a check in jj2lsj and stop with an informative warning text for such lists?

@jongrumer jongrumer added the enhancement New feature or request label May 20, 2019
@CYChenFudan
Copy link
Contributor

Lines around "
do LS_number = 1, asf_set_LS%csf_set_LS%nocsf
if ((asf_set_LS%csf_set_LS%csf(LS_number)%parity == "+" &
!Changed by cychen
!.and. ISPAR(iw1) == 1) .or. &
.and. ISPAR(NCFMIN) == 1) .or. &
(asf_set_LS%csf_set_LS%csf(LS_number)%parity == "-" &
!Changed by cychen
!.and. ISPAR(iw1) == -1)) then
.and. ISPAR(NCFMIN) == -1)) then
" to deal with both parities.

@CYChenFudan
Copy link
Contributor

Lines 220 and 222 in jj2lsj_code.f90 should be modified.

@jongrumer
Copy link
Member Author

jongrumer commented May 20, 2019

Thank you @CYChenFudan! Let's see what @gaigalas thinks! We want to be sure that this works, and does not have an effect on other connected routines such as the radiative transition codes.

@CYChenFudan
Copy link
Contributor

@jongrumer You are welcome. By the way, we also modified the codes to deal with He-like ions (including both 1snl and 2lnl configurations), and the large-scaled transfermation (the indices of array exceeding 2^31). I am wondering how to upload these modifications.

@jongrumer
Copy link
Member Author

@jongrumer You are welcome. By the way, we also modified the codes to deal with He-like ions (including both 1snl and 2lnl configurations), and the large-scaled transfermation (the indices of array exceeding 2^31). I am wondering how to upload these modifications.

Ok! Sounds interesting! The standard procedure is to:

  1. Create a new branch from the master branch named with your initials + topic, i.e. for example: "CYC/He-large-scale". You can do this directly in the online interface, or locally. Make sure you start off from the latest master version.
  2. Add all your changes as logically separated and well-explained commits on this new branch. Try to stay synced with master as you work on this new development branch. This makes life easier when you're finally going to merge with master.
  3. When you're happy and the modified code works as expected, make sure you test it carefully, create a pull request here in the online github interface, where you ask for your development branch to be merged into the master branch.
  4. With the pull request active, it's then possible for the rest of the team to test and discuss your suggestions. At some point we will decide (me currently) if the pull request should be merged or not.

It all sounds a bit complicated (if you haven't done it before) but in reality it's actually not that bad.
The github help pages have a lot of info on this standard workflow.

Great to see more people joining the development! I'll come to Shanghai on ASOS, we can discuss more then!

Jon

@CYChenFudan
Copy link
Contributor

OK. I'll try. Let's discuss at Shanghai.

@AhmedSiddiq995
Copy link

Please help me out with the following error in jj2lsj program
Program received signal SIGSEGV: Segmentation fault - invalid memory reference.

Backtrace for this error:
#0 0x7f18add6832a
#1 0x7f18add67503
#2 0x7f18ad5fcf1f
#3 0x7f18adedcb8b
#4 0x7f18adeddcbb
#5 0x7f18adee10a8
#6 0x7f18adee272c
#7 0x564dc56bc3ac
#8 0x564dc56be025
#9 0x564dc56ad3ee
#10 0x7f18ad5dfb96
#11 0x564dc56ad419
#12 0xffffffffffffffff
Segmentation fault (core dumped)

Regards
Ahmed

@jongrumer
Copy link
Member Author

jongrumer commented Sep 23, 2020

The ability to run both parities at the same time is addressed in Chen's patch in PR #45.

@CYChenFudan Does this patch also include your enhancement for large indices related to the He-calculations you adressed in the comment above? Or did you decide to leave this for the future?

"By the way, we also modified the codes to deal with He-like ions (including both 1snl and 2lnl configurations), and the large-scaled transfermation (the indices of array exceeding 2^31)."

@CYChenFudan
Copy link
Contributor

@jongrumer No, the other enhancements are not yet included in F90-version. We'll find some time for them in the future.

@jongrumer jongrumer linked a pull request Sep 30, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants