Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAJOR UPDATE: Remove CCS, VINC #292

Merged
merged 31 commits into from
Oct 23, 2023
Merged

MAJOR UPDATE: Remove CCS, VINC #292

merged 31 commits into from
Oct 23, 2023

Conversation

AlexTMallen
Copy link
Collaborator

This PR does several major things to the repository:

  • It removes support for CCS and VINC, and removes the use of contrast pairs
  • The InferenceServer class is used to do inference, enabling FSDP (--fsdp) inference with much larger models
    • It also only requires loading the model once, and doesn't load the model if the dataset is cached
  • Only support LR probes and LM predictions
  • LM predictions are taken in response to statement + suffix where statement is the statement we're extracting hidden states from and suffix is a piece of text like "\n\nIs the above statement true or false?" that's optionally specified in the yaml template file
  • logprobs are optionally stored with --save_logprobs
  • support for non-balanced datasets with --balance False
  • Added testing for:
    • The pipeline that turns the raw HF dataset into the input_ids dataset passed to InferenceServer
    • InferenceServer
  • default template called "_default" which takes the "statement" column itself

Copy link
Member

@norabelrose norabelrose left a comment

Choose a reason for hiding this comment

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

almost good to go

@@ -185,7 +190,12 @@ def fit_cv(

@classmethod
def inlp(
Copy link
Member

Choose a reason for hiding this comment

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

I think we should probably just get rid of this. If we really want to explore the half-space of admissible predictors we should probably try the Conical Knowledge Decomposition thing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have actually been using this occasionally, basically it just gives me a higher recall way of searching for good probes. Do you still think we should get rid of it?

elk/training/sweep.py Outdated Show resolved Hide resolved
"""Whether to use LEACE to erase the paraphrase dimensions before training the
classifier."""

max_inlp_iter: int | None = None
Copy link
Member

Choose a reason for hiding this comment

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

let's drop this

elk/extraction/extraction.py Outdated Show resolved Hide resolved
elk/extraction/generator.py Outdated Show resolved Hide resolved
elk/promptsource/templates.py Outdated Show resolved Hide resolved
elk/plotting/visualize.py Outdated Show resolved Hide resolved
@@ -185,7 +190,12 @@ def fit_cv(

@classmethod
def inlp(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have actually been using this occasionally, basically it just gives me a higher recall way of searching for good probes. Do you still think we should get rid of it?

elk/training/sweep.py Outdated Show resolved Hide resolved
…args, fix answer token being appended, fix viz, fix tqdm propagation
Copy link
Member

@norabelrose norabelrose left a comment

Choose a reason for hiding this comment

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

let's merge this bad boy

@norabelrose norabelrose merged commit 70a3290 into main Oct 23, 2023
4 checks passed
@norabelrose norabelrose deleted the no-contrast-pairs branch October 23, 2023 01:18
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.

2 participants