-
Notifications
You must be signed in to change notification settings - Fork 526
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
add InputNlist
into API doc
#1009
Conversation
This struct is necessary for C++ APIs.
Codecov Report
@@ Coverage Diff @@
## devel #1009 +/- ##
==========================================
+ Coverage 74.59% 82.86% +8.26%
==========================================
Files 86 119 +33
Lines 6921 10110 +3189
==========================================
+ Hits 5163 8378 +3215
+ Misses 1758 1732 -26
Continue to review full report at Codecov.
|
source/api_cc/include/common.h
Outdated
std::vector<int > numneigh; | ||
/// Array stores the core region atom's neighbor index |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this comment is for numneigh
. Place it above std::vector<int > numneigh;
?
The dsame issue for ilist
and jlist
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In #968, I see @denghuilu add the same comment to InputNlist::firstneigh
. Isn't it the same thing as NeighborListData::firstneigh
? Did I understand wrongly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think jlist stores the same thing as firstneigh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@denghuilu @njzjz jlist
stores the indexes of all the neighbors. firstneigh[i]
stores the location of the first neighbor of i
. So the neighbors of i
are stored in jlist[firstneigh[i] : firstneigh[i] + numneigh[i]
@denghuilu Please check again you code you really use the neighbor list correctly....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@amcadmus I think you are right, so the NeighborListData is used to construct the InputNlist. From my point of view, firstneigh is a two-dimensional array that stores neighbor atom's index, even if it only refers to a series of jlist locations. I'm using the InputNlist in the custom op, so I think it's fine.
source/api_cc/include/common.h
Outdated
std::vector<int > numneigh; | ||
/// Array stores the core region atom's neighbor index |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think jlist stores the same thing as firstneigh
Co-authored-by: Denghui Lu <[email protected]>
* add `InputNlist` into API doc This struct is necessary for C++ APIs. * Apply suggestions from code review Co-authored-by: Denghui Lu <[email protected]> * update comment for `firstneigh` Co-authored-by: Denghui Lu <[email protected]>
This struct is necessary for C++ APIs. Without document, no one can understand this struct...
@Yi-FanLi I believe you need it.
Preview below: