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

DFT: Adds OctreeGridBlocker -> BlockOPoints pruning check. #1525

Merged
merged 2 commits into from Feb 8, 2019

Conversation

Projects
None yet
4 participants
@jturney
Copy link
Member

jturney commented Feb 6, 2019

Description

BlockOPoints performs additional pruning but the result is never checked. We recently hit a case where this pruning resulted in a BlockOPoints object with zero points. This would cause a segfault here: https://github.com/psi4/psi4/blob/master/psi4/src/psi4/libfock/v.cc#L262 because ncols would be zero.

Adds a check to OctreeGridBlocker where the list of BlockOPoints is populated, if the number of points is zero then don't add it to the list.

Checklist

Status

  • Ready for review
  • Ready for merge

@jturney jturney added the bug label Feb 6, 2019

@loriab

loriab approved these changes Feb 6, 2019

@andysim

andysim approved these changes Feb 6, 2019

Copy link
Member

andysim left a comment

Good catch! LGTM

@jturney

This comment has been minimized.

Copy link
Member Author

jturney commented Feb 6, 2019

Don't merge this just yet. I'm checking to see if https://github.com/psi4/psi4/pull/1525/files#diff-0699760b6ecbe2dbea8a9ca91f1979b1R4412 needs to be executed regardless.

@jturney

This comment has been minimized.

Copy link
Member Author

jturney commented Feb 6, 2019

All the tests still pass when the index is still offset. Looking at the code it seems logical to continue offsetting the index as that is what is used when creating the next block of points.

@loriab loriab added this to the Psi4 1.3 milestone Feb 7, 2019

@dgasmith dgasmith merged commit 331d912 into psi4:master Feb 8, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
psi4.psi4 #20190206.3 succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.