Repository navigation
fix: per-module aggregation in result analysis with several hookpoints - #181
Open
flyingdoubleG wants to merge 1 commit into
Open
flyingdoubleG wants to merge 1 commit into
flyingdoubleG wants to merge 1 commit into
Conversation
- load_data globbed '*{module}*', so with hookpoints 'layers.1' and
'layers.10' the score files of layers.10 were also loaded, and
counted, as layers.1. Match '{module}_latent*', the score file name.
- frequency_weighted_f1 indexed the firing-count tensor with a pandas
Series. Torch reads it positionally (series[0]) while pandas looks up
labels, so every module after the first raised KeyError: 0 and
log_results crashed for runs with several hookpoints.
- The firing count was set only when 'latent_idx in counts[module]', a
test on the tensor's values; check the index range instead.
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.
Summary
Three problems in
delphi/log/result_analysis.py(run at the end of the pipeline with--verbose). The first two show up when several hookpoints are analysed.load_datacollects files withglob(f"*{module}*"). Score files are namedf"{module}_latent{idx}.txt"(Latent.__str__), so with--hookpoints layers.1 layers.10the pattern*layers.1*also matcheslayers.10_latent*.txt. Those rows are loaded a second time, labelledlayers.1, and weighted bylayers.1's firing counts. Residual-stream SAEs such asEleutherAI/sae-llama-3-8b-32xuse hookpoints namedlayers.N.frequency_weighted_f1raisesKeyError: 0for every module after the first.counts[module][module_df["latent_idx"]]indexes a tensor with a pandas Series: torch reads the sequence positionally (series[0]), while pandas looks up the label. Only the first module's rows have labels starting at 0, solog_resultscrashes for any run with more than one hookpoint.firing_countis mostlyNone. It is set onlyif latent_idx in counts[module], which tests whether some count value equalslatent_idx, not whether the index is in range. This column feedsplot_firing_vs_f1.Changes
f"{module}_latent*", with/replaced by--as when the files are written.torch.as_tensor(module_df["latent_idx"].to_numpy()).latent_idx < len(counts[module]).Tests
Added to
tests/test_log/test_result_analysis.py, using detection scores for latent 0 oflayers.1(all correct) andlayers.10(all wrong):test_load_data_keeps_modules_with_shared_prefix_apart: each module gets its own 2 rows, andfiring_countis read by index. On main,layers.1gets 4 rows.test_frequency_weighted_f1_with_several_modules:weighted_f1 == 0.5, the mean of the per-module F1s 1.0 and 0.0. On main this raisesKeyError: 0.Both fail on main and pass with this change.
ruff check(with the repo's excludes) andblack --checkpass. Locally I ran the tests withouttests/conftest.pybecause vLLM does not install on Windows.