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

Add Support for Returning Logits for Generated Tokens #637

Merged

Conversation

Kyle1668
Copy link
Contributor

@Kyle1668 Kyle1668 commented Jul 1, 2022

Closes #588

@Kyle1668 Kyle1668 requested a review from a team as a code owner July 1, 2022 15:49
@Kyle1668 Kyle1668 force-pushed the Kyle1668/get_logits_from_generation branch from c678846 to a0ed80b Compare July 3, 2022 00:40
@Kyle1668 Kyle1668 force-pushed the Kyle1668/get_logits_from_generation branch from b25b59a to a43cf77 Compare July 3, 2022 00:45
@Kyle1668 Kyle1668 changed the title [Draft] Add Support for Returning Logits for Generated Tokens Add Support for Returning Logits for Generated Tokens Jul 3, 2022
@Kyle1668
Copy link
Contributor Author

Kyle1668 commented Jul 3, 2022

The original issue mentioned potentially returning the log-likelihood of the tokens through some integration in the generate.py interface. I decided against implementing that support in this PR and instead returned the raw logits. My reasoning is that end users can then operate on these logits as needed.

@Kyle1668
Copy link
Contributor Author

Kyle1668 commented Jul 3, 2022

Outputs from ./deepy.py generate.py ./configs/20B.yml ./configs/text_generation.yml on my branch

With return_logits: true

image

without

{
  "context": "",
  "text": "Entry DescriptionWild Agnes has released this extraordinary new",
  "length": 10,
  "finished": false,
  "message": null,
  "duration_seconds": 6.626634120941162
}

on main

{
  "context": "",
  "text": "Entry DescriptionWild Agnes has released this extraordinary new",
  "length": 10,
  "finished": false,
  "message": null,
  "duration_seconds": 6.523743629455566
}

@Kyle1668
Copy link
Contributor Author

Kyle1668 commented Jul 3, 2022

I took a stab at writing some unit tests for this change but ran into an issue with mpu.get_model_parallel_src_rank() throwing an exception saying that I needed to configure init_process_group.

This was referenced Jul 3, 2022
Copy link
Member

@StellaAthena StellaAthena left a comment

Choose a reason for hiding this comment

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

This is great! Apologies that this PR took so long to review and merge, but we hugely appreciate this work.

@StellaAthena StellaAthena merged commit 87d01ad into EleutherAI:main Sep 8, 2022
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.

Restore ability to get logits from generation?
2 participants