fix: Ensure activation scaling factor set before initializing b_dec #440
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.
Description
Fixes #439
When
b_dec_init_method
is set tomean
orgeometric_median
we require getting activations from the buffer, howeverset_norm_scaling_factor_if_needed
was not being called prior to this, meaning ifnormalize_activations
was set toexpected_average_only_in
it would throw an error.I added a call to
set_norm_scaling_factor_if_needed
directly before accessing the activations buffer inside_init_sae_group_b_decs
to ensure the scaling factor is set before initialization. I also added a condition toset_norm_scaling_factor_if_needed
to check whether the scaling factor is already set to prevent it from being set twice.An alternative solution would be to take this opportunity to resolve the TODO for
_init_sae_group_b_decs
and move that method to another class, which I'm happy to do as well.Type of change
Checklist:
You have tested formatting, typing and tests
make check-ci
to check format and linting. (you can runmake format
to format code if needed.)