Skip to content

fix dplr: correct type check in get_valid_pairs#2580

Merged
wanghan-iapcm merged 17 commits intodeepmodeling:develfrom
Yi-FanLi:fix_dplr_dpl_type
Jun 5, 2023
Merged

fix dplr: correct type check in get_valid_pairs#2580
wanghan-iapcm merged 17 commits intodeepmodeling:develfrom
Yi-FanLi:fix_dplr_dpl_type

Conversation

@Yi-FanLi
Copy link
Collaborator

@Yi-FanLi Yi-FanLi commented May 31, 2023

The binary_search method in C++ is only valid for sorted vectors.
The following case in "type_associate" of "fix dplr" will raise error: 4 5 3 6. 4 and 3 are real atom types and stored in sel_type. The vector sel_type is sorted. 5 and 6 are associated with 4 and 3, respectively. The resulting sel_type vector is 3 4, and the resulting dpl_type vector is 6 5. If a pair 4 5 is searched, then this line

binary_search(dpl_type.begin(), dpl_type.end(),
dtype[bondlist[ii][1]])) {

will do binary search for 4 in 3 4, which results in True; and will search for 5 in 6 5, which results in False.

Therefore, I am copying dpl_type vector to the dpl_type_sorted vector and sort it. The binary search should be used on the sorted vector.

The above solution is abandoned.

In stead, do not use binary_search in the type check of get_valid_pairs.
The current method is: first find the type of the real atom, and check if the type of its bonded atom is consistent with that defined by type_associate.

@codecov
Copy link

codecov bot commented May 31, 2023

Codecov Report

Patch coverage: 48.27% and project coverage change: -0.02 ⚠️

Comparison is base (0d732ad) 76.67% compared to head (ff1de55) 76.66%.

Additional details and impacted files
@@            Coverage Diff             @@
##            devel    #2580      +/-   ##
==========================================
- Coverage   76.67%   76.66%   -0.02%     
==========================================
  Files         233      233              
  Lines       24158    24175      +17     
  Branches     1710     1697      -13     
==========================================
+ Hits        18524    18533       +9     
- Misses       4507     4519      +12     
+ Partials     1127     1123       -4     
Impacted Files Coverage Δ
source/lmp/fix_dplr.cpp 77.74% <48.27%> (-1.84%) ⬇️

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Yi-FanLi Yi-FanLi requested a review from njzjz June 2, 2023 15:26
@Yi-FanLi Yi-FanLi changed the title Use a sorted dpl_type vector in binary search fix dplr: correct type check in get_valid_pairs Jun 5, 2023
@wanghan-iapcm wanghan-iapcm enabled auto-merge (squash) June 5, 2023 03:51
@wanghan-iapcm wanghan-iapcm merged commit 48bcd13 into deepmodeling:devel Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants