Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

fixing reasoning perturbation issues #4883

Merged
merged 2 commits into from
Nov 17, 2022
Merged

fixing reasoning perturbation issues #4883

merged 2 commits into from
Nov 17, 2022

Conversation

Golovneva
Copy link
Contributor

Patch description
Fixing some issues with perturbations:

  1. Math selecting empty steps to insert as hallucination. Ideally, we would want to remove this empty steps at tokenization, but that would change all our MATH data and affect all scores. So instead this patch.
  2. In proofwriter, use only those hallucinated steps that are not part of the context
  3. In Grammar, compare with tokenized step, to avoid situation where chains are identical everywhere except some random spaces
  4. Fix the name of the finetuned BART model to align with what was uploaded to S3

@Golovneva Golovneva requested a review from moyapchen November 16, 2022 02:48
moyapchen
moyapchen approved these changes Nov 17, 2022
View reviewed changes
Copy link
Contributor

@moyapchen moyapchen left a comment

Choose a reason for hiding this comment

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

LGTM

parlai/tasks/math_dataset/agents.py Outdated
random_step = self.math_random.choice(rand_steps)
random_step = ""
# find non-empty random step
while random_step == "" or random_step == " ":
Copy link
Contributor

Choose a reason for hiding this comment

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

Any chance this could infinite loop (cause all steps are "", in case someone tries to mix perturbations)?

parlai/tasks/proof_writer/agents.py
# make sure new step is from a different context
while not random_step or random_step in m["question"]:
rand_steps = self.proofwriter_random.choice(messages)["steps"]
random_step = self.proofwriter_random.choice(rand_steps)
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto to question above about infinite loops

@Golovneva Golovneva merged commit 4ffb29c into main Nov 17, 2022
@Golovneva Golovneva deleted the olggol/perturb-fix branch November 17, 2022 16:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@moyapchen moyapchen moyapchen approved these changes

Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Golovneva @moyapchen @facebook-github-bot