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.

Sign up for GitHub

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

Jump to bottom

Adding the model input iterators. #806

Closed
wants to merge 12 commits into from
Closed

Adding the model input iterators. #806

wants to merge 12 commits into from

Conversation

xuzhao9
Copy link
Contributor

@xuzhao9 xuzhao9 commented Mar 16, 2022 •
edited
Loading

This PR adds a new interface, gen_inputs(self) -> Tuple[Generator, Optional[int]]. The first element is an iterator of the input data, and the second element is the size of the iterator. If the iterator is infinite, the second value is set to None.

Currently, only implement this on timm and torchvision models. Both of them use randomly generated inputs, therefore the second value is set to None in both cases.

@xuzhao9 xuzhao9 changed the title Adding the model input iterators. [WIP] Adding the model input iterators. Mar 17, 2022
@xuzhao9 xuzhao9 force-pushed the xz9/add-input-iterators branch 2 times, most recently from 4fc4658 to 76fe5b9 Compare March 25, 2022 21:06
@xuzhao9 xuzhao9 changed the title [WIP] Adding the model input iterators. Adding the model input iterators. Mar 25, 2022
@facebook-github-bot
Copy link
Contributor

@xuzhao9 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@shunting314
Copy link
Contributor

The interface I feel more convenient is:

get_inputs(self, n_needed: int) -> List[ExampleBatch]

When I test the model for n iterations, I'll call get_inputs(self, n) to get n batches and feed each batch to each run.

But I think the interface you provided is also fine, since I can wrap around it.

@shunting314
Copy link
Contributor

Do we know what are the list of models that this interface can be implemented?

@xuzhao9
Copy link
Contributor Author

xuzhao9 commented Mar 25, 2022

Do we know what are the list of models that this interface can be implemented?

I believe they can be implemented for all models in https://github.com/pytorch/benchmark/tree/main/torchbenchmark/models, but some of them reads data from a mini-dataset, so the length of inputs is limited (for example, n will be 10 or something since we don't have more batches).
For others (notably torchvision/timm/huggingface), since the input is randomly generated, we can generate infinite number of inputs

@shunting314
Copy link
Contributor

Got it. What's the plan to implement this method for other models in torchbench?

@xuzhao9
Copy link
Contributor Author

xuzhao9 commented Mar 25, 2022

Got it. What's the plan to implement this method for other models in torchbench?

It will need some time to implement, I expect to reach 100% coverage by the end of April

@facebook-github-bot
Copy link
Contributor

@xuzhao9 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@xuzhao9 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@xuzhao9 xuzhao9 mentioned this pull request Apr 8, 2022
Merged
@xuzhao9 xuzhao9 deleted the xz9/add-input-iterators branch April 8, 2022 18:23
gairgeio added a commit to gairgeio/benchmark that referenced this pull request Aug 2, 2024
@gairgeio @facebook-github-bot
Adding the model input iterators. (#806)
5ed78d6 
Summary:
This PR adds a new interface, `gen_inputs(self) -> Tuple[Generator, Optional[int]]`. The first element is an iterator of the input data, and the second element is the size of the iterator. If the iterator is infinite, the second value is set to None.

Currently, only implement this on `timm` and `torchvision` models. Both of them use randomly generated inputs, therefore the second value is set to None in both cases.

Pull Request resolved: pytorch/benchmark#806

Reviewed By: shunting314

Differential Revision: D35155417

Pulled By: xuzhao9

fbshipit-source-id: 5e60223279b09ed4336996f7292741be96070bdd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shunting314 shunting314 shunting314 approved these changes

@Krovatkin Krovatkin Awaiting requested review from Krovatkin

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
@xuzhao9 @facebook-github-bot @shunting314