Skip to content

Conversation

@ibhati
Copy link
Member

@ibhati ibhati commented Feb 4, 2026

Add IVF static and dynamic index in cpp runtime

@ethanglaser ethanglaser mentioned this pull request Feb 6, 2026
Copy link
Contributor

@ahuber21 ahuber21 left a comment

Choose a reason for hiding this comment

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

Very nice! I have just a few nitpicks that would be easy to sort out.


// Dispatch on storage kind for Dynamic IVF operations (uses blocked allocator)
template <typename F, typename... Args>
auto dispatch_ivf_blocked_storage_kind(StorageKind kind, F&& f, Args&&... args) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see this used anywhere. Will a blocked index be added separately?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed

CATCH_REQUIRE(status.ok());

status = index->compact();
CATCH_REQUIRE(status.ok());
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe also check for smaller size after compacting?

Copy link
Member Author

Choose a reason for hiding this comment

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

We have compact related unit tests in the library

Copy link
Contributor

Choose a reason for hiding this comment

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

This entire file would benefit from a few svs::runtime::v0::... statements for better readability.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am just following the existing structure, this could be a separate PR for better readability in vamana/ivf tests

ethanglaser added a commit that referenced this pull request Feb 10, 2026
Pending merge of #269 , #251 , and maybe #267
@ibhati ibhati marked this pull request as ready for review February 11, 2026 17:52
@intel intel deleted a comment from parishwilliejerome-max Feb 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants