feat: support MRLE with split RaBitQ

[LightWant]

Change Type

• Bug fix
• New feature
• Improvement/Refactor
• Documentation
• CI/Build/Infra

Linked Issue

• Fixes: [feat](other): support dimension reduction with split RaBitQ in HGraph and Pyramid #2533

What Changed

• allow RaBitQ split datacells to access TransformQuantizer<RaBitQuantizer> through a generic bottom-quantizer accessor
• support the exact mrle,rabitq split configuration in HGraph and Pyramid while keeping split encoding in the terminal RaBitQ quantizer
• share split parameter mapping and validation while preserving direct RaBitQ split behavior
• fix reduced-dimension TransformQuantizer training layout and strengthen compatibility checks
• add unit, functional, and English/Chinese documentation coverage

Test Evidence

• make fmt
• make lint
• make test
• make cov, run tests, and collect coverage
• Other (describe below)

VSAG_ENABLE_LIBAIO=OFF make debug COMPILE_JOBS=4

Regression tests:
- complete unit suite: 639 cases, 85,539,431 assertions
- MRLE unit: 3 cases, 71 assertions
- MRLE functional: 2 cases, 210 assertions
- RaBitQ split direct compute: 4096 assertions
- RaBitQ split optimized scalar build: 1656 assertions
- TransformQuantizer compute: 92592 assertions
- HGraph direct mapping, Pyramid precise mapping, and TransformQuantizer compatibility passed

Dataset evaluation: openai-1536-50k-angular.hdf5
- mrle_dim=768: Recall@10 0.7640, QPS 358.83, average latency 2.783 ms (single thread)
- mrle_dim=1536 control: Recall@10 0.9941, QPS 238.33, average latency 4.192 ms

The complete unit suite passed. The complete functional suite was started and manually stopped during a long-running Daily case after the affected-path functional tests had passed; no pre-interrupt failure was observed.

Compatibility Impact

• API/ABI compatibility: additive only
• Behavior changes: exact tq_chain="mrle,rabitq" can now use RaBitQ x+y split in HGraph and Pyramid; other transformed split chains remain rejected

Performance and Concurrency Impact

• Performance impact: reduced dimensions lower code size and search cost, with a dataset-dependent recall tradeoff
• Concurrency/thread-safety impact: no public concurrency model change; optimized-build finalization continues using disjoint code slots

Documentation Impact

• No docs update needed
• Updated docs:
  • README.md
  • DEVELOPMENT.md
  • CONTRIBUTING.md
  • Other: English/Chinese quantization, HGraph, and index parameter docs

Risk and Rollback

• Risk level: medium
• Rollback plan: revert this PR

Checklist

• The code follows the project style and has been formatted
• New behavior is covered by tests
• Documentation is updated where needed
• The commit includes a DCO sign-off

[mergify]

Merge Protections

🟢 All 3 merge protections satisfied — ready to merge.

Show 3 satisfied protections

🟢 Require kind label

• label~=^kind/

🟢 Require version label

• label~=^version/

🟢 Require linked issue for feature/bug PRs

• body~=(?im)(?:^|[\s\-\*])(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s*:?\s+(?:#\d+|[\w.\-]+/[\w.\-]+#\d+|https?://github\.com/[\w.\-]+/[\w.\-]+/issues/\d+)

[vsag-bot]

/label status/waiting-for-review
/waiting-on reviewer
/request-review @jiaweizone
/request-review @wxyucs
/request-review @inabao

[LightWant]

Maintainers: please add the required kind/feature and version/1.1 labels. The contributor account does not have permission to apply repository labels.

[LHT129]

Review Summary

This PR adds MRLE transform quantizer support to RaBitQ split datacells, enabling the "mrle, rabitq" TQ chain for both HGraph and Pyramid index types. The implementation is well-structured and follows existing patterns in the codebase.

What Changed

• New BottomQuantizerAccessor template: A zero-overhead abstraction that peels through TransformQuantizer<RaBitQuantizer> to access the inner quantizer, computer, and transformed input vectors
• RaBitQ split datacell refactoring: Uses BottomQuantizerAccessor throughout instead of direct quantizer access, making it work with both direct RaBitQuantizer and TransformQuantizer<RaBitQuantizer>
• Factory dispatch: MakeRaBitQSplitDataCell now accepts is_transform_quantizer flag; when true, validates the TQ chain is exactly "mrle, rabitq" and instantiates the correct template
• Shared parameter mapping: MapRaBitQSplitParam and ValidateMRLEDim extracted from HGraph-specific code to shared inner_index_parameter.cpp, now used by both HGraph and Pyramid
• Pyramid reorder_by_base support: New reorder_by_base_ mode where reorder uses base_codes_ directly (needed when base codes are RaBitQ split codes)
• TransformQuantizer additions: GetTransformedDim() and TransformBaseVector() methods for use by the accessor
• Comprehensive tests: Unit tests for the accessor pattern, integration tests for Pyramid and HGraph MRLE+RaBitQ split flows

Key Observations

1. The BottomQuantizerAccessor pattern is clean and zero-cost at compile time
2. The ExecuteChainTransform method has a pre-existing issue where memcpy always copies this->dim_ floats after each transform step, which is incorrect after MRLE dimensionality reduction — this is not introduced by this PR but is now exposed through TransformBaseVector
3. Test coverage is solid across unit and integration levels
4. The factory validation correctly rejects unsupported TQ chains

Verdict

The PR is well-implemented with good abstraction, proper validation, and thorough testing. No blocking issues found.

[vsag-bot]

Automated pull request review completed.

Review effort: high (1014 changed lines across 31 files).

Submitted 4 inline comments.
Review: #2560 (review)

[LHT129]

Code Review Summary

I have completed a thorough review of all 30 changed files in this PR. The existing inline comments from the previous reviewer have already covered the major design points well. Here is my assessment:

Architecture & Design

• BottomQuantizerAccessor pattern: Clean template specialization that correctly handles both direct RaBitQuantizer and TransformQuantizer<RaBitQuantizer> cases. Zero-overhead at compile time.
• QuantizerT template parameter on RaBitQSplitDataCell: Defaults to RaBitQuantizer<metric>, preserving backward compatibility for all existing direct split usage. The TQ path instantiates with TransformQuantizer<RaBitQuantizer<metric>, metric>.
• Split of responsibilities: Train/Encode/FactoryComputer/Serialize use the outer quantizer_ (which may be TQ-wrapped), while split/merge/lower-bound/full-distance operations use bottom_quantizer() via the accessor. This is the correct separation.

Correctness

• TrainImpl fix: Changing from this->dim_ to GetTransformedDim() for the output buffer size is the key correctness fix. Without it, MRLE-reduced vectors would have stride mismatches after the first vector.
• ExecuteChainTransform fix: Using vector_transformer->GetOutputDim() instead of this->dim_ for the memcpy size after each transform step. This was a pre-existing bug that is now fixed.
• PyramidParameters::CheckCompatibility: Correctly skips precise_codes_param comparison when reorder_source == "base", since precise_codes_param is null in that case.
• TransformQuantizerParameter::CheckCompatibility: Now compares both transformer parameters AND bottom quantizer parameters, which is more thorough than the previous type-name-only comparison.
• Pyramid::CalcDistanceById: Uses graph_codes() which correctly returns base_codes_ when reordering from base. The calculate_precise_distance parameter becomes a no-op in that case, which is semantically correct since there is no separate precise store.

Validation

• MapRaBitQSplitParam: Validates both direct (rabitq) and MRLE (tq with "mrle, rabitq" chain) cases. The is_mrle branch correctly validates the exact chain.
• MakeRaBitQSplitDataCellForMetric: Factory-level validation that TQ split only accepts "mrle, rabitq". This is a defense-in-depth check.
• ValidateMRLEDim: Properly extracted as a shared function, validates [0, dim] range.

Test Coverage

• Unit tests: MRLE+split factory creation, batch training layout, optimized build with base transform, split query vs merged full-code consistency, invalid chain rejection, parameter mapping, TQ compatibility.
• Functional tests: HGraph MRLE+RaBitQ split build+KNN, Pyramid MRLE+RaBitQ split build+KNN.
• Direct split regression tests preserved.

Documentation

• Both English and Chinese docs updated for quantization_transform.md, hgraph.md, index_parameters.md.
• Implementation plan document (issue_2533_mrle_split_rabitq_implementation_plan.md) provides clear context.

No Blocking Issues Found

All changes are consistent with the codebase style, the validation is thorough, and the test coverage is adequate. The PR is well-structured and ready to merge.

[vsag-bot]

Automated inline review completed.

Review effort: high (1012 changed lines across 30 files).
Submitted 3 inline comments.

Reviewed commit 36331b9.

[vsag-bot]

Automated inline review completed.

Review effort: high (1181 changed lines across 32 files).
Submitted 4 inline comments.

Reviewed commit dc08162.

[vsag-bot]

[P2] Retain vectors for every accepted Pyramid TQ configuration

The new mapping accepts base_quantization_type="tq" even when split bits are absent, but this predicate retains raw vectors only for rabitq_split. For example, tq_chain="mrle,fp32" with use_reorder=false reaches NSW FLAT-node promotion, where decodable_codes() returns the TQ base cell; its Decode always returns false and Build throws Pyramid graph promotion requires decodable vectors. Either restrict Pyramid's public TQ mapping to the supported split chain or retain a decodable source for every accepted TQ configuration.

[vsag-bot]

[P2] Compare reorder sources during Pyramid compatibility checks

This branches only on the current parameter's reorder_source and never compares it with the serialized parameter. A streaming index built with precise reorder can therefore be accepted by an otherwise identical base-reorder instance; loading skips HIGH_PRECISION_CODES, then search and reorder use base codes even though the serialized graph was built with precise codes. Compare reorder_source before selecting which code parameters to validate.

[vsag-bot]

[P2] Keep a decodable source for HGraph MRLE split statistics

Enabling this TQ split branch sets reorder to base and eliminates the precise cell, but HGraph does not automatically retain raw vectors. With the default store_raw_vector=false, HGraphAnalyzer samples through GetVectorByInnerId; TransformQuantizer::Decode returns false and HGraph ignores that result, leaving zero-filled samples and producing meaningless GetStats() ground-truth and neighbor metrics. Retain a raw/decodable source for this configuration or make analysis operate without decoding.

[vsag-bot]

[P3] Document the new parameters on the Pyramid pages

This advertises Pyramid as supporting external TQ and MRLE split configuration, but the canonical English and Chinese Pyramid pages still omit tq from base_quantization_type and do not document tq_chain, mrle_dim, reorder_source, or the split-bit parameters. Users following those pages cannot construct the newly supported configuration or understand its raw-vector storage and recall tradeoffs; update both Pyramid pages and their index-parameter reference sections.

[LHT129]

[suggestion] The requires_raw_vector predicate is too narrow — it only triggers for RABITQ_SPLIT_CODES + QUANTIZATION_TYPE_VALUE_TQ. If a future quantizer also needs raw vectors (e.g., any quantizer wrapped in TransformQuantizer that cannot decode to fp32), this check will silently skip storing them. Consider using a more general predicate like is_transform_quantizer or checking quantizer->CanDecodeToFp32() if such an API exists.

[LHT129]

[suggestion] CheckCompatibility does not compare reorder_source between two parameter sets. If one index was built with reorder_source="base" and another with reorder_source="precise", they would be considered compatible despite having fundamentally different data layouts (one stores raw vectors, the other does not). This could cause silent correctness issues during merge or reload. Add a CHECK_FIELD_EQ(*this, *p, reorder_source) here.

[LHT129]

[suggestion] HGraph MRLE split does not set store_raw_vector (unlike Pyramid at pyramid.cpp:1067). This means HGraph with MRLE split cannot decode vectors to fp32 for statistics or debugging. If this is intentional (HGraph does not need raw vectors), consider adding a comment explaining why. Otherwise, mirror the Pyramid logic to set store_raw_vector when MRLE split is detected.

[LHT129]

Code Review Summary

Reviewed PR #2560: feat: support MRLE with split RaBitQ (commit dc08162).

What this PR does

Adds MRLE (Multi-Resolution Local Encoding) dimensionality reduction support to RaBitQ split quantization in both HGraph and Pyramid indexes. Key changes:

• BottomQuantizerAccessor: Template abstraction to unwrap TransformQuantizer<RaBitQuantizer> for split datacell
• RaBitQSplitDataCell: Now templated on QuantizerT, works with both direct RaBitQuantizer and TransformQuantizer wrapper
• Pyramid: New reorder_by_base_ mode, decodable_codes()/graph_codes() helpers, raw vector storage for MRLE split
• Shared utilities: MapRaBitQSplitParam and ValidateMRLEDim extracted to InnerIndexParameter

Issues identified in this review

1. [suggestion] requires_raw_vector predicate in pyramid.cpp:1067 is too narrow — only triggers for specific type strings
2. [suggestion] CheckCompatibility in pyramid_zparameters.cpp:295 does not compare reorder_source
3. [suggestion] HGraph MRLE split does not set store_raw_vector (unlike Pyramid)
4. [note] Documentation could be expanded for the new reorder_source + MRLE split interaction

Positive observations

• Clean extraction of shared MapRaBitQSplitParam/ValidateMRLEDim to InnerIndexParameter
• BottomQuantizerAccessor is a well-designed template abstraction
• ExecuteChainTransform fix for dimensionality reduction is correct
• Test coverage added for the new parameter paths

[LHT129]

[suggestion] TransformBaseVector allocates a heap buffer on every call.

The method at transform_quantizer.h:278-283 creates a Vector<float> data_buffer(this->dim_, ...) on every invocation. In the optimized-build path (rabitq_split_datacell.h:1088-1090), this is called for every inserted vector, causing repeated heap allocations.

Consider one of:

1. Accept a pre-allocated scratch buffer as a parameter (caller already has transformed_input).
2. Make the scratch buffer a static thread_local to reuse across calls within the same thread.

The caller in write_encoded_vector already has a Vector<float> transformed_input that could be sized to dim_ and passed in, avoiding the internal allocation entirely.

[LHT129]

[suggestion] TrainImpl now calls TransformBaseVector in a per-element loop, each allocating a temporary buffer.

At transform_quantizer.h:248-251, the training loop calls TransformBaseVector for each of count vectors. Each call internally allocates a Vector<float> data_buffer(this->dim_, ...). For large training sets this adds unnecessary allocation overhead.

The previous code allocated one Vector<float> transformed_data(transformed_dim * count, ...) and mutated it in-place via ExecuteChainTransform. The new approach is cleaner but regresses on allocation count.

Consider extracting the per-element transform logic into a method that accepts a pre-allocated scratch buffer, or hoisting the scratch allocation outside the loop:

Vector<float> scratch(this->dim_, 0, this->allocator_);
for (uint64_t i = 0; i < count; ++i) {
    scratch.assign(data + i * this->dim_, data + (i + 1) * this->dim_);
    ExecuteChainTransform(scratch.data(), nullptr, nullptr);
    memcpy(transformed_data.data() + i * transformed_dim, scratch.data(), transformed_dim * sizeof(float));
}

[LHT129]

[suggestion] CheckCompatibility does not compare reorder_source between two parameter sets.

At pyramid_zparameters.cpp:294-297, the compatibility check branches on this->reorder_source to decide whether to compare precise_codes_param, but it never compares this->reorder_source against p->reorder_source.

This means an index built with reorder_source="precise" (has precise_codes_ with a separate fp32 cell) would be considered compatible with one built with reorder_source="base" (no precise_codes_, reorders from base_codes_), despite fundamentally different data layouts.

Add a CHECK_FIELD_EQ(*this, *p, reorder_source); before the conditional branch at line 295.

[LHT129]

[suggestion] The requires_raw_vector predicate is too narrow — it only triggers for RABITQ_SPLIT_CODES + QUANTIZATION_TYPE_VALUE_TQ.

At pyramid.cpp:1065-1068, the condition:

const bool requires_raw_vector =
    inner_json[BASE_CODES_KEY][CODES_TYPE_KEY].GetString() == RABITQ_SPLIT_CODES and
    inner_json[BASE_CODES_KEY][QUANTIZATION_PARAMS_KEY][TYPE_KEY].GetString() ==
        QUANTIZATION_TYPE_VALUE_TQ;

only retains raw vectors for the MRLE+RaBitQ split combination. If a future TQ chain (e.g., "pca, rabitq" with split storage) also produces non-decodable codes, this check will silently skip storing raw vectors, causing GetVectorByInnerId and FLAT-to-GRAPH promotion to fail at runtime.

Consider inverting the logic: check whether the base quantizer supports decoding (e.g., base_codes_->GetQuantizer()->SupportDecode() or similar), and retain raw vectors whenever it does not. This makes the predicate future-proof against any non-decodable quantizer configuration.

[LHT129]

[suggestion] HGraph MRLE split does not set store_raw_vector, unlike Pyramid.

At hgraph_param_mapping.cpp:559, CheckAndMappingExternalParam calls MapRaBitQSplitParam and ValidateMRLEDim but does not set store_raw_vector when MRLE+split is detected. Pyramid does this at pyramid.cpp:1065-1068.

HGraph with MRLE split cannot decode vectors to fp32 for statistics (HGraphAnalyzer) or GetVectorByInnerId. If this is intentional (HGraph does not need raw vectors because it never promotes FLAT nodes or decodes), consider adding a comment explaining why. Otherwise, add the same requires_raw_vector logic as Pyramid.

The HGraphAnalyzer path at hgraph_analyzer.cpp calls GetVectorByInnerId which will throw INTERNAL_ERROR if the base codes cannot decode.

[LHT129]

Overall Review

This PR adds MRLE + RaBitQ x+y split support for both HGraph and Pyramid indexes. The architecture is well-designed: BottomQuantizerAccessor cleanly abstracts the TransformQuantizer wrapper, shared parameter mapping is extracted to inner_index_parameter.cpp, and Pyramid gains graph_codes()/decodable_codes() accessors that correctly handle the new reorder_by_base_ and raw_vector_ paths.

What is done well

• Clean abstraction: BottomQuantizerAccessor with primary template + TQ specialization keeps the split datacell code generic.
• Shared parameter mapping: MapRaBitQSplitParam and ValidateMRLEDim are extracted from HGraph-specific code into shared utilities used by both HGraph and Pyramid.
• Correct training layout fix: TrainImpl now uses GetTransformedDim() for buffer sizing, fixing a bug where reduced-dimension transforms would write past valid data.
• ExecuteChainTransform dim fix: Uses vector_transformer->GetOutputDim() for memcpy, fixing the same class of bug in the encode path.
• Good test coverage: Unit tests for MRLE split in flatten_datacell_test.cpp, pyramid_test.cpp, pyramid_zparameters_test.cpp, and integration tests in test_pyramid.cpp and test_hgraph_rabitq_split.cpp.
• TransformQuantizerParameter::CheckCompatibility now correctly delegates to both VectorTransformerParameter and the bottom quantizer parameter, fixing a gap where transform parameters (e.g., mrle_dim) were not compared.

Key concerns

1. TransformBaseVector per-call heap allocation (transform_quantizer.h:280): Creates a Vector<float> on every call, including in the optimized-build hot path. The caller already has a scratch buffer that could be reused.
2. TrainImpl per-element allocation (transform_quantizer.h:248): The training loop calls TransformBaseVector for each vector, each allocating internally. A single scratch buffer outside the loop would be more efficient.
3. CheckCompatibility missing reorder_source comparison (pyramid_zparameters.cpp:295): Two parameter sets with different reorder_source values are considered compatible despite fundamentally different data layouts.
4. requires_raw_vector predicate too narrow (pyramid.cpp:1065): Hard-coded to RABITQ_SPLIT_CODES + QUANTIZATION_TYPE_VALUE_TQ. A future non-decodable quantizer would silently skip raw vector storage.
5. HGraph missing store_raw_vector (hgraph_param_mapping.cpp:559): Unlike Pyramid, HGraph does not retain raw vectors for MRLE split, which may cause HGraphAnalyzer to fail.

Verdict

The PR is well-structured and the core logic is correct. The concerns above are primarily about future-proofing and performance optimization — none are blocking correctness issues for the current MRLE+RaBitQ split use case. Recommend addressing items 3 and 5 before merge (correctness gaps), and items 1 and 2 as follow-up optimizations.

[vsag-bot]

Automated inline review completed.

Review effort: high (1014 changed lines across 31 files).
Submitted 4 inline comments.

Reviewed commit 9aa97c8.

[vsag-bot]

[P2] Retain raw vectors for HGraph MRLE split

This newly accepts MRLE split while forcing reorder to the non-decodable base TQ cell, but HGraph still defaults store_raw_vector to false. GetVectorByInnerId ignores the failed TQ Decode, so default MRLE-split indexes feed zero vectors into GetStats() and the non-forced UpdateVector checks, producing meaningless statistics and potentially incorrect update decisions. Enable a raw/decodable source automatically for this configuration.

[vsag-bot]

[P2] Retain vectors for every accepted Pyramid TQ configuration

Pyramid now maps arbitrary tq_chain values, but raw vectors are retained only for split RaBitQ. For example, base_quantization_type="tq", tq_chain="mrle,fp32", use_reorder=false, and index_min_size=3 is accepted; when the third item promotes the FLAT node, decodable_codes() returns the TQ base cell, whose Decode always fails, and Add throws. Either restrict Pyramid's new public TQ mapping to the supported split chain or retain a decodable source for every accepted TQ layout that lacks decodable precise codes.

[vsag-bot]

[P2] Compare reorder sources before accepting serialized Pyramid parameters

Compatibility branches only on this instance's reorder_source and never compares it with p->reorder_source. Otherwise identical use_reorder=true indexes using precise and base reorder are therefore accepted despite different serialized layouts; binary loading can interpret the stored precise-code block as hierarchy data, while streaming loading skips that block and searches a graph built with a different distance source. Compare reorder_source before selecting which code parameters to validate.

[vsag-bot]

[P3] Document the new Pyramid TQ parameters on Pyramid pages

This now advertises Pyramid as publicly supporting TQ and MRLE split, but the canonical English and Chinese Pyramid pages still omit tq from base_quantization_type and do not document tq_chain, mrle_dim, reorder_source, or the split-bit parameters. Users following the Pyramid documentation cannot construct the advertised configuration or understand its raw-vector storage and recall tradeoff; update both Pyramid pages and their index-parameter reference section.