Skip to content

Conversation

@aknayar
Copy link
Contributor

@aknayar aknayar commented Oct 30, 2025

This PR adds IndexFlatL2Panorama, integrating Panorama (as specified in the paper) into IndexFlatL2. This is the first step in creating an IndexRefinePanorama, which will use IndexFlatL2Panorama (or an IndexPreTransform with an IndexFlatL2Panorama) as its refine_index.

Refactoring

Since the bulk of Panorama's refinement logic would be duplicated between IndexFlatL2Panorama and IndexIVFFlatPanorama, it has been factored out into a new Panorama struct. This struct contains key parameters (batch_size, d, etc.) and the following utility functions:

  • copy_codes_to_level_layout: Writes new vectors to codes following Panorama's storage layout
  • compute_cumulative_sums: Computes the cumulative sums for new vectors
  • compute_query_cum_sums: Computes the cumulative sums for a new query
  • progressive_filter_batch: Performs Panorama refinement on a batch of vectors

These utilities will be shared by most Panorama indexes, which is why I have refactored them into their own utility.

IndexRefinePanorama

While the IndexFlatL2Panorama implemented in this PR technically contains all the functionality needed to implement IndexRefinePanorama (performing search on a subset of indices), it is not ready to be used as a refine_index. The current implementation is not optimized for the case of IndexRefine, where we perform search on a very small subset of the datapoints. This leads to vastly scattered memory accesses during the search, to the point where the overhead of maintaining active_indices and exact_distances can thwart Panorama's speedups.

As such, to optimize for IndexRefine we will need a standalone implementation of search_subset which instead does the following:

  1. Iterate over the subset of indices (rather than batches of codes)
  2. For each index i, compute its distance alone by Panorama refinement (essentially having batch_size = 1. In fact, for this very reason I have made batch_size a parameter in the constructor—IndexRefine will require it to be 1 due to noncontiguous memory accesses, but typical workloads would benefit from 128-1024.)

This will unfortunately mean we cannot reuse the search utilities in the Panorama struct in this specific case, but will allow us to squeeze 2-5x speedups during the reordering phase of IndexRefine.

Testing

  1. Unit tests can be found in tests/test_flat_l2_panorama.py
  2. A benchmark can be found in benchs/bench_flat_l2_panorama.py, yielding the following results:
======Flat
        Recall@10: 0.980000, speed: 263.214254 ms/query, dims scanned: 100.00%
======PCA960,FlatL2Panorama8_512
        Recall@10: 0.980000, speed: 37.080264 ms/query, dims scanned: 12.62%

The recall being less than 1.0 is perhaps due to discrepancies between faiss results and the ground_truth values.
bench_flat_l2_panorama

@meta-cla meta-cla bot added the CLA Signed label Oct 30, 2025
"""Test when n_levels doesn't evenly divide dimension"""
test_cases = [(65, 4), (63, 8), (100, 7)]

# TODO(aknayar): Test functions like get_single_code().
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will add these tests in a follow-up PR.

@aknayar
Copy link
Contributor Author

aknayar commented Oct 30, 2025

I rebuilt with AVX2 on Linux and was unable to reproduce the failing tests seen here, any ideas what may have happened?

@limqiying
Copy link
Contributor

Hey, let me just copy what @mnorris11 said and we can resume the thread from there.

When checking the log, it is just 2 elements wrong in one unit test. Some ideas:

There could be a tie in distance, and different ids happen to get returned. If this is the case, they could update the test where if there is a mismatch, then check distances, and if they are equal then still do not fail the test.

Do you have faiss installed with numpy2? It's a recent integration, and that could be the reason for the difference. Let me know the conda steps you took to repro!

@aknayar
Copy link
Contributor Author

aknayar commented Oct 31, 2025

@limqiying Thank you for the ideas! It seems like it was, in fact, some weird case involving a tie. Panorama also suffers from floating-point imprecision due to how we calculate squared L2 norm: Faiss does $\Vert\ q - p\ \Vert_2^2$ while Panorama does $\Vert\ q\Vert_2^2 + \Vert p\Vert_2^2 - 2(q\cdot p)$. As a result, such cases where ties produce different results are possible (albeit rare). Simply changing the random seed on the dataset generator for the testcase seemed to fix things!

@aknayar aknayar marked this pull request as draft November 3, 2025 15:58
@aknayar aknayar marked this pull request as ready for review November 3, 2025 16:05
n_levels(n_levels),
batch_size(batch_size),
pano(code_size, n_levels, batch_size) {
FAISS_THROW_IF_NOT(metric == METRIC_L2);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be relaxed when we add inner product support.

cum_sums.clear();
}

void IndexFlatPanorama::reconstruct(idx_t key, float* recons) const {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can implement these in a follow-up PR since this one is quite big already.

@meta-codesync
Copy link
Contributor

meta-codesync bot commented Nov 4, 2025

@limqiying has imported this pull request. If you are a Meta employee, you can view this in D86234773.

@limqiying limqiying requested a review from mdouze November 4, 2025 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants