Skip to content

feature: save and load in flat index#163

Merged
yuejiaointel merged 21 commits into
mainfrom
feature_flat_index_save_load
Aug 6, 2025
Merged

feature: save and load in flat index#163
yuejiaointel merged 21 commits into
mainfrom
feature_flat_index_save_load

Conversation

@yuejiaointel
Copy link
Copy Markdown
Contributor

Add save and load functions to flat index

@yuejiaointel yuejiaointel changed the title fix: add simple save feature: save and load in flat index Aug 5, 2025
@yuejiaointel yuejiaointel marked this pull request as ready for review August 5, 2025 05:45
@ahuber21
Copy link
Copy Markdown
Contributor

ahuber21 commented Aug 5, 2025

Awesome, looks like a lot was already there, especially the assemble. Let's wait for comments from @ibhati

Comment thread bindings/python/src/flat.cpp
Comment thread bindings/python/tests/test_flat.py Outdated
Comment thread include/svs/index/flat/flat.h
Comment thread include/svs/index/flat/flat.h Outdated
Comment thread tests/svs/index/flat/flat.cpp Outdated
@yuejiaointel yuejiaointel requested a review from ibhati August 5, 2025 23:44
Comment thread bindings/python/src/flat.cpp Outdated
data_directory: Directory where the dataset will be saved.

Note: All directories should be separate to avoid accidental name collision with any
auxiliary files that are needed when saving the various components of the index.
Copy link
Copy Markdown
Member

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 Note as there is only one directory in the Flat index unlike the Vamana index, which saves config/graph/data in multiple directories

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thx, removed!

Comment thread bindings/python/tests/test_flat.py Outdated
print("Flat, From Array, Float32")
flat = svs.Flat(data_f32, svs.DistanceType.L2)
self._do_test(flat, queries_f32, groundtruth, svs.DistanceType.L2, data_f32)
save_reload_and_test(flat, queries_f32, groundtruth, data_f32)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
save_reload_and_test(flat, queries_f32, groundtruth, data_f32)
self._do_test(flat, queries_f32, groundtruth, svs.DistanceType.L2, data_f32)
save_reload_and_test(flat, queries_f32, groundtruth, data_f32)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please keep the original _do_test(...) call before doing save/load. The same comment applies to other calls below

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thx, fixed by have save_and_load and a remove_dir called before and after _do_test

Comment thread tests/integration/exhaustive.cpp Outdated
svs::VectorDataLoader<float>(temp_dir), distance_type, index.get_num_threads()
);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe do this in the end of this function followed by search and recall testing

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thx, moved to end of the function!

Copy link
Copy Markdown
Member

@ibhati ibhati left a comment

Choose a reason for hiding this comment

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

@yuejiaointel Thanks for incorporating the feedback. Now, it looks great to me!

@yuejiaointel yuejiaointel merged commit 91b0816 into main Aug 6, 2025
11 checks passed
@yuejiaointel yuejiaointel deleted the feature_flat_index_save_load branch August 6, 2025 20:01
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.

3 participants