fix: preserve c-TF-IDF idf diagonal in delete_topics (#2530) - #2538
Open
kiruthick01 wants to merge 1 commit into
Open
fix: preserve c-TF-IDF idf diagonal in delete_topics (#2530)#2538kiruthick01 wants to merge 1 commit into
kiruthick01 wants to merge 1 commit into
Conversation
`delete_topics` mutated `ctfidf_model._idf_diag` as if it were indexed by topic, both when adding the -1 topic and when removing deleted topics. The idf diagonal is a (n_features, n_features) matrix over the vocabulary and does not depend on the number of topics, so these mutations left it non-square. Any later call relying on `ctfidf_model.transform` then failed, e.g. `topics_over_time` raising "ValueError: matmul: dimension mismatch" after a topic was deleted. Leave `_idf_diag` unchanged in `delete_topics`; the topic-indexed attributes are still updated as before. Add a regression test covering models with and without an existing -1 topic. Fixes MaartenGr#2530
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is wrong
After a model has had topics removed with
delete_topics,topics_over_time(and any other path that relies onctfidf_model.transform) raises:This is #2530.
Root cause
ClassTfidfTransformer._idf_diagis a(n_features, n_features)diagonal matrix over the vocabulary (built inClassTfidfTransformer.fitassp.diags(idf, shape=(n_features, n_features))), andtransformapplies it asX = X * self._idf_diag. It is independent of the number of topics.delete_topics, however, mutated it as if it were indexed by topic, in two places:-1topic for the first time, it prepended a row:Both change the row count of a matrix whose rows are vocabulary features, not topics, leaving
_idf_diagnon-square soX * _idf_diagfails. (When exactly one topic is deleted from a model that had no-1yet, the+1/-1happen to cancel in shape, but the diagonal is then silently misaligned with the vocabulary.)Reproduction
What changed
delete_topicsno longer modifiesctfidf_model._idf_diag. Deleting topics (or adding the-1outlier) does not change the vocabulary, so the idf diagonal is left as-is. The genuinely topic-indexed attributes (topic_embeddings_,c_tf_idf_, representations, representative docs/images, etc.) are still updated exactly as before.Tests
Added
test_delete_topics_preserves_ctfidf_idf_diag, parametrized overkmeans_pca_topic_model(no-1) andbase_topic_model(with-1). It asserts_idf_diagstays square and unchanged in shape afterdelete_topics, and thattopics_over_timesucceeds afterwards. The test fails onmaster(_idf_diagbecomes non-square, e.g.(18365, 18367) != (18367, 18367)) and passes with this change.Validation
pytest tests/test_reduction/test_delete.py tests/test_variations/test_dynamic.py→ 13 passedruff checkandruff format --checkon the changed files → cleanFixes #2530.
I kept the change minimal and focused on the idf-diagonal handling. Happy to adjust the approach or the test based on your preference.