Fix wrong output size in SplineC2ROMPTarget::mw_evaluateVGLandDetRatioGrads #4408
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fix SplineC2R. When the requested size is smaller than what can be supplied, only output the requested size. What is the right way of selecting a subset of orbitals in an already built SplineC2R remains a big question. The trucation is subject to the sorting of orbitals in C2R which puts all the "complex" orbitals first. "complex" orbitals produce two real orbitals.
ye-luo
commented
Jan 23, 2023
| const size_t first_cplx = first / 2; | ||
| const size_t last_cplx = omptarget::min(last / 2, orb_size); |
There was a problem hiding this comment.
The min() here is wrong. orb_size refers to output real orbitals.
| ValueType* restrict out_dphi_x = out_phi + phi_vgl_stride; | ||
| ValueType* restrict out_dphi_y = out_dphi_x + phi_vgl_stride; | ||
| ValueType* restrict out_dphi_z = out_dphi_y + phi_vgl_stride; | ||
| ValueType* restrict out_d2phi = out_dphi_z + phi_vgl_stride; | ||
|
|
||
| const size_t first_real = first_cplx + omptarget::min(nComplexBands_local, first_cplx); | ||
| const size_t last_real = last_cplx + omptarget::min(nComplexBands_local, last_cplx); |
There was a problem hiding this comment.
last_real can be larger than requested_orb_size and caused overflow.
|
Test this please |
prckent
reviewed
Jan 23, 2023
Because we don't have a test case exposing this bug. The added test triggers asan failure when I backported it to develop. |
|
Test this please |
|
I also noticed that asan runs on the github actions without QMC_DATA. We probably need to update the image to include NiO a4(S1) and a16(S4) h5 files. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
There is bug when using offload real build.
This caused writing to undesired memory locations and wrong numbers (crazy high variance/energy >>1)
What type(s) of changes does this code introduce?
Does this introduce a breaking change?
What systems has this change been tested on?
epyc-server
Checklist