Skip to content

Read pooling_mode_mean_tokens with get() so a partial config does not raise - #1188

Open
Aryan-Pardeshi wants to merge 1 commit into
neuml:masterfrom
Aryan-Pardeshi:fix-partial-pooling-config
Open

Read pooling_mode_mean_tokens with get() so a partial config does not raise#1188
Aryan-Pardeshi wants to merge 1 commit into
neuml:masterfrom
Aryan-Pardeshi:fix-partial-pooling-config

Conversation

@Aryan-Pardeshi

Copy link
Copy Markdown
Contributor

Closes #1187

PoolingFactory.method() guarded every other pooling flag with .get() but read pooling_mode_mean_tokens with a bracket lookup, so a 1_Pooling/config.json that sets a mode without that key raised KeyError out of PoolingFactory.create() instead of resolving a method:

with patch.object(PoolingFactory, "load", return_value={"pooling_mode_cls_token": True}):
    PoolingFactory.method("any/model")   # KeyError: 'pooling_mode_mean_tokens'

A missing flag now reads as disabled, matching an explicit false and the defensive style used by the rest of the module (config and config.get(...), maxlength()'s if config guard, load() swallowing DownloadError).

Tests

testPartialPoolingConfig covers the resolution table both ways — cls/last with the mean key absent and present-and-false both resolve to their method, mean-enabled and an empty config resolve to meanpooling.

Note on #1186

#1186 adds a third flag check on the same lines (pooling_mode_max_tokens). Whichever of the two lands second I'll bring in line — it's a one-token change either way.

… raise

PoolingFactory.method() guarded every other pooling flag with .get() but
looked up pooling_mode_mean_tokens with a bracket, so a 1_Pooling/config.json
that sets a mode without that key raised KeyError out of
PoolingFactory.create() instead of resolving a method.

A missing flag now reads as disabled, matching an explicit false.

Closes neuml#1187
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.

PoolingFactory.method raises KeyError on a pooling config without pooling_mode_mean_tokens

1 participant