Skip to content

Filter elements with an optional filtering function #402

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.

Sign up for GitHub

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

Merged
merged 12 commits into from
Sep 6, 2022
Merged

Filter elements with an optional filtering function #402

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
765c4ab
Filter elements with an optional filtering function.
kishorenc Aug 15, 2022
ad3440c
Filter function should be sent the label and not the internal ID.
kishorenc Aug 19, 2022
4f6dcc3
Ensure that results are not empty when reading from top results.
kishorenc Aug 19, 2022
1c833a7
Make allowAllIds static.
kishorenc Aug 25, 2022
aaee13a
Use functor for filtering.
kishorenc Aug 26, 2022
b87f623
Explicitly check for filter functor being default.
kishorenc Aug 27, 2022
f0dedf3
Remove duplicate assignment.
kishorenc Aug 28, 2022
de22860
Merge branch 'develop' into filter-elements
kishorenc Aug 28, 2022
e4705fd
Add search with filter test to CI.
kishorenc Aug 28, 2022
7f419ea
Remove constexpr for functor in test.
kishorenc Aug 28, 2022
1fe7baf
Add check for is_filter_disabled.
kishorenc Sep 6, 2022
c9897b0
Add assert header.
kishorenc Sep 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions hnswlib/hnswalg.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,8 @@ namespace hnswlib {
std::priority_queue<std::pair<dist_t, tableint>, std::vector<std::pair<dist_t, tableint>>, CompareByFirst> candidate_set;

dist_t lowerBound;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add the following micro optimisation to not call isIdAllowed at all if filtering is disabled:

bool is_filter_disabled = isIdAllowed == allowAllIds;
if ((!has_deletions || !isMarkedDeleted(ep_id)) && (is_filter_disabled || isIdAllowed(getExternalLabel(ep_id)))) {

if ((!has_deletions || !isMarkedDeleted(ep_id)) && isIdAllowed(getExternalLabel(ep_id))) {
bool is_filter_disabled = std::is_same<filter_func_t, decltype(allowAllIds)>::value;
if ((!has_deletions || !isMarkedDeleted(ep_id)) && (is_filter_disabled || isIdAllowed(getExternalLabel(ep_id)))) {
dist_t dist = fstdistfunc_(data_point, getDataByInternalId(ep_id), dist_func_param_);
lowerBound = dist;
top_candidates.emplace(dist, ep_id);
Expand Down Expand Up @@ -307,7 +308,8 @@ namespace hnswlib {
_MM_HINT_T0);////////////////////////
#endif

if ((!has_deletions || !isMarkedDeleted(candidate_id)) && isIdAllowed(getExternalLabel(candidate_id)))
is_filter_disabled = std::is_same<filter_func_t, decltype(allowAllIds)>::value;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove this line as above we assigned value to the is_filter_disabled variable.

if ((!has_deletions || !isMarkedDeleted(candidate_id)) && (is_filter_disabled || isIdAllowed(getExternalLabel(candidate_id))))
top_candidates.emplace(dist, candidate_id);

if (top_candidates.size() > ef)
Expand Down
2 changes: 1 addition & 1 deletion hnswlib/hnswlib.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ namespace hnswlib {
bool operator()(Args&&...) { return true; }
};

FilterFunctor allowAllIds;
static FilterFunctor allowAllIds;

template <typename T>
class pairGreater {
Expand Down