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

LCAO Calculation with 0 down-spin electron - Seg fault #945

Closed
anbenali opened this issue Jul 31, 2018 · 8 comments · Fixed by #2148
Closed

LCAO Calculation with 0 down-spin electron - Seg fault #945

anbenali opened this issue Jul 31, 2018 · 8 comments · Fixed by #2148
Assignees

Comments

@anbenali
Copy link
Contributor

anbenali commented Jul 31, 2018

When using ECP, Cases like Na, Li or other elements end up with one electron only in the valence. Normal converter and all attempts to remove the down determinant lead to a segmentation fault or an error in the LCAO builder. Does anyone remember/know what is the "trick" to get these systems to run properly.

@anbenali anbenali added the bug label Jul 31, 2018
@prckent
Copy link
Contributor

prckent commented Jul 31, 2018

Just to confirm: you have no issue with the converter(s), only QMCPACK?

There should be no trick needed of course - it is totally reasonable to run a single electron of "either" spin, but probably this is a corner case we need to check.

Do you get a SEGV with no jastrow?

@ye-luo
Copy link
Contributor

ye-luo commented Jul 31, 2018

Please provide an reproduce of this issue. Just attach files here.

@anbenali
Copy link
Contributor Author

Yes. Converter generates correct file (in the sens that it avoids generating 2BJ and 3Body J and puts the size of the down determinant to 0 but I get a segfault when running qmcpack. This is definetly a corner case but quite annoying for isolated atoms such as Li Na or H (even if I never run H).

@prckent
Copy link
Contributor

prckent commented Jul 31, 2018

It is a bug, possibly several.

@anbenali
Copy link
Contributor Author

Na.tar.gz

@ye-luo
Copy link
Contributor

ye-luo commented Aug 1, 2018

A quick investigation shows multiple breakage in the code.

@prckent prckent added this to the V3.5.1 Release milestone Aug 1, 2018
@PDoakORNL PDoakORNL self-assigned this Aug 1, 2018
@prckent prckent modified the milestones: V3.5.1 Release, V3.6.0 Release Nov 7, 2018
@PDoakORNL PDoakORNL added the to do label Feb 7, 2019
@prckent prckent modified the milestones: V3.7.0 Release, V3.8.0 Release Mar 15, 2019
@ye-luo
Copy link
Contributor

ye-luo commented Jul 9, 2019

To support systems with electrons all in one kind of spin. The converter writes the electron particleset with size 0 of down spin electron and a determinant of size 0.

  1. Currently, the code complains that the number of orbitals doesn't meet the requirement >0.
  2. After bypassing the check and allows 0, I got a segmentation fault. Because, the code expect two determinants based on the number of groups (2) in the particleset but only one was generated. The down electron determinant was skipped
  3. Then remove the return and got an error in the delayed update input check. default delay rank is 1 and it doesn't meet the requirement [1, number of electrons].
  4. After bypassing the check, I got an MKL error that dimension input of 0 is not acceptable by MKL.
  5. Should I continue putting tons if(>0) in the source code?

Change my strategy.

  1. Remove the 0 size down spin in the particleset of input.
  2. Remove the 0 size down spin determinant in the determinantset of input.
  3. Viva. I got the right result.

So in theory, there are two ways to support this case. One doesn't mention the second spin at all. The other one fakes the second spin with size 0. If we want to fix this feature, we should stop allowing the second way.

If we decide to stop user input if the sizes are 0. We will need to protect all the converters. I think this is doable. However, it is not clear to me how to deal with multi determinant. I believe the current multi-det code never worked with system with only 1 kind of spin.

@prckent
Copy link
Contributor

prckent commented Jul 10, 2019

Thanks for looking at this. Updating the converters seems straightforward and, equally important, maintainable.

For the main application, not mentioning the down spin determinant at all in the inputs is preferable since it is cleaner from the user perspective, and we should need very few if (n>0) checks throughout the code. Aborting if a zero size determinant is specified in the input is straightforward.

Running a multidet calculation with only electrons of one spin present seems like a case we don't need to worry about immediately. If the multidet code has problems, make a follow-up issue. Logically it shouldn't be too hard to fix (...), but I think it likely that other more important parts of the code could have troubles, e.g. in the optimizers.

@ye-luo ye-luo changed the title LCAO Calculation with only 1 electron - Seg fault LCAO Calculation with 0 down-spin electron - Seg fault Dec 12, 2019
@ye-luo ye-luo modified the milestones: On Deck, v3.9.0 Release Dec 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants