Skip to content

[fix] Write the sparsity CSV columns once in three sparse evaluators - #3985

Merged
tomaarsen merged 2 commits into
huggingface:mainfrom
MaxFreedomPollard:fix-sparse-evaluator-csv-headers
Sep 7, 2026
Merged

tomaarsen merged 2 commits into
huggingface:mainfrom
MaxFreedomPollard:fix-sparse-evaluator-csv-headers

Conversation

@MaxFreedomPollard

Copy link
Copy Markdown
Contributor

SparseBinaryClassificationEvaluator, SparseEmbeddingSimilarityEvaluator and SparseTripletEvaluator write a CSV whose header has more columns than its rows, so every value ends up under the wrong column name. It is the same bug as #3512, which you fixed for SparseInformationRetrievalEvaluator in #3456 but which is still there in these three.

Each of them overrides _append_csv_headers to append its active_dims / sparsity_ratio columns (sparse_binary_classification.py:136, sparse_embedding_similarity.py:114, sparse_triplet.py:129). The dense superclass calls that hook twice whenever similarity_fn_names is left unset, which is the default: once from __init__ with an empty list, and again from __call__ once model.similarity_fn_name has filled it in (binary_classification.py:121 and :180, embedding_similarity.py:132 and :187, triplet.py:157 and :196). The metric columns are only added on the second call, but the sparse columns were appended unconditionally, so they landed in csv_headers both times.

The row does not grow to match, because the metric values come from the scores dict and the sparsity values are appended once by append_to_last_row. SparseTripletEvaluator on main writes a 15 column header over a 9 column row:

epoch,steps,anchor_active_dims,anchor_sparsity_ratio,positive_active_dims,positive_sparsity_ratio,negative_active_dims,negative_sparsity_ratio,accuracy_dot,anchor_active_dims,anchor_sparsity_ratio,positive_active_dims,positive_sparsity_ratio,negative_active_dims,negative_sparsity_ratio
-1,-1,0.5,77.25,0.9974690387261648,102.75,0.9966335757814035,102.75,0.9966335757814035

The fix is the guard SparseInformationRetrievalEvaluator._append_csv_headers and SparseNanoBEIREvaluator._append_csv_headers already use: append the sparse columns only when the superclass is adding metric columns too. Passing similarity_fn_names explicitly was already fine and stays fine, since then only the __init__ call adds anything.

The new test builds every SparseEncoder evaluator that writes a CSV and checks the header has no repeated column and exactly as many cells as the row.

$ pytest tests/sparse_encoder/test_evaluators.py -q
3 failed, 4 passed        # on main: binary_classification, embedding_similarity, triplet
7 passed                  # with this change
$ pytest tests/sparse_encoder tests/sentence_transformer/evaluation -q
442 passed, 2 skipped in 306s
$ pre-commit run --files <the four files touched>
ruff check Passed, ruff format Passed, typos Passed

I also checked the CSVs the other sparse evaluators write, before and after: SparseInformationRetrievalEvaluator, SparseNanoBEIREvaluator, SparseRerankingEvaluator, SparseTranslationEvaluator and SparseMSEEvaluator are unaffected, and repeated calls appending to an existing file stay aligned.

MaxFreedomPollard and others added 2 commits September 6, 2026 12:29
SparseBinaryClassificationEvaluator, SparseEmbeddingSimilarityEvaluator and
SparseTripletEvaluator each override _append_csv_headers to append their
active_dims / sparsity_ratio columns. Their dense superclasses call that hook
twice whenever similarity_fn_names is left unset, which is the default: once
from __init__ with an empty list, and again from __call__ once
model.similarity_fn_name has filled it in. The metric columns are only added on
the second call, but the sparse columns were appended unconditionally, so they
landed in csv_headers both times.

The row does not grow to match. The metric values come from the scores dict and
the sparsity values are appended once by append_to_last_row, so the header ended
up two columns wider than every row (six, for the triplet evaluator) and every
value was filed under the wrong column name. This is the same bug as #3512,
fixed for SparseInformationRetrievalEvaluator in #3456 but not for these three.

Apply the guard that SparseInformationRetrievalEvaluator._append_csv_headers and
SparseNanoBEIREvaluator._append_csv_headers already use: only append the sparse
columns when the superclass is also adding metric columns.
@tomaarsen

Copy link
Copy Markdown
Member

Hello!

Thanks for the fix! I've moved the tests into tests/sparse_encoder/evaluation/ to match the existing layout and focused them on the three affected evaluators. They now cover default and explicit similarity functions, repeated calls, and CSV values matching their columns.

  • Tom Aarsen

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The fix is narrowly scoped, matches established patterns in similar evaluators, and is covered by targeted regression tests that validate CSV header/row alignment.

Pull request overview

This PR fixes a CSV serialization bug in sparse evaluators where sparse-specific columns were being appended to csv_headers twice (when similarity_fn_names is left unset), causing header/row misalignment and duplicated header columns. The change aligns these evaluators with the existing guard pattern already used in other sparse evaluators.

Changes:

  • Guard sparse-specific header appends in SparseBinaryClassificationEvaluator, SparseEmbeddingSimilarityEvaluator, and SparseTripletEvaluator so they only run when similarity_fn_names is non-empty (i.e., when metric columns are also being added).
  • Add regression tests that run each evaluator twice and assert the written CSV header matches the expected columns and aligns exactly with written row values.
  • Add tests/sparse_encoder/evaluation/__init__.py (for package/annotation consistency in the new test directory).
File summaries
File Description
sentence_transformers/sparse_encoder/evaluation/sparse_binary_classification.py Only append active_dims / sparsity_ratio headers when metric headers are appended.
sentence_transformers/sparse_encoder/evaluation/sparse_embedding_similarity.py Only append active_dims / sparsity_ratio headers when metric headers are appended.
sentence_transformers/sparse_encoder/evaluation/sparse_triplet.py Only append per-role sparsity headers when metric headers are appended.
tests/sparse_encoder/evaluation/test_binary_classification_evaluator.py Regression test: header/row alignment for sparse binary classification evaluator CSV.
tests/sparse_encoder/evaluation/test_embedding_similarity_evaluator.py Regression test: header/row alignment for sparse embedding similarity evaluator CSV.
tests/sparse_encoder/evaluation/test_triplet_evaluator.py Regression test: header/row alignment for sparse triplet evaluator CSV.
tests/sparse_encoder/evaluation/init.py Initializes the new test package (future annotations).
Review details
  • Files reviewed: 7/7 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@tomaarsen
tomaarsen enabled auto-merge (squash) September 7, 2026 07:57
@tomaarsen
tomaarsen merged commit 729f28e into huggingface:main Sep 7, 2026
10 checks passed
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