Fix handling of high l-momentum channels in ECP local potential #2920
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
|
Can one of the admins verify this patch? |
ye-luo
requested changes
Feb 15, 2021
prckent
changed the title
|
I changed the title to make writing the human readable changelog easier. |
ye-luo
reviewed
Feb 15, 2021
| @@ -154,7 +154,7 @@ void ECPComponentBuilder::buildLocal(xmlNodePtr cur) | |||
| { | |||
| if (grid_global == 0) | |||
| { | |||
There was a problem hiding this comment.
Please remove {} for a single line. Please update all the places touched.
| @@ -32,13 +32,13 @@ void ECPComponentBuilder::buildSemiLocalAndLocal(std::vector<xmlNodePtr>& semiPt | |||
| if (grid_global == 0) | |||
| { | |||
| app_error() << " Global grid needs to be defined." << std::endl; | |||
There was a problem hiding this comment.
app_error() is more for printing from any rank which is unknown.
Since the error is uniform. Please
fuse
app_error() << " Global grid needs to be defined." << std::endl;
into myComm->barrier_and_abort to print one copy only.
| rmax = std::max(rmax, rc); | ||
| if (angMon.find(lstr) == angMon.end()) | ||
| { | ||
| app_error() << "requested angular momentum " << lstr << " not available.\n"; |
There was a problem hiding this comment.
examples
You may use ostringstream to buffer contents if it is long.
|
Okay to test |
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.
Please review the developer documentation
on the wiki of this project that contains help and requirements.
Proposed changes
This PR closes issue #2917.
ECPComponentBuilder had a str,int map for allowed angular momentum, and only had up to "g".
Since the NonLocalECPComponent has implemented nonlocal channels up to lmax = 7 (j channel), I added to the angMon map up to lmax = 8 (k channel) since the k channel could be used as a local in that case.
Additionally, in ECPComponentBuidler::buildSemiLocalAndLocal I add a check to see if the requested angular momentum is actually in the angMon map. Otherwise it aborts
What type(s) of changes does this code introduce?
Delete the items that do not apply
Does this introduce a breaking change?
What systems has this change been tested on?
My workstation
Checklist
Update the following with a yes where the items apply. If you're unsure about any of them, don't hesitate to ask. This is
simply a reminder of what we are going to look for before merging your code.