-
Notifications
You must be signed in to change notification settings - Fork 157
Support to Generate Fake Sample/Inputs #1180
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall. Left a few comments
@rahul-tuli thanks a lot for the fix! Nit: why do we save exported samples at generic location |
One more nit: when models are pushed to SparseZoo, these files should be in the top directory of (pinging @anmarques in case I've missed something) |
…` supplied in export script
That is a great suggestion @eldarkurtic, I've made a note of this and will put this up as a follow up PR after discussing with the team, Thank you! |
Done! 🥃 tmp_trainer ls
runs sample-inputs sample-outputs |
Simplify call to `_get_fake_inputs` Save inputs/outputs to `sample-inputs`/`sample-outputs`
29a42c7
to
ea179c1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, few nits
@rahul-tuli @eldarkurtic afaik the standard as we speak is |
@dbogunowicz the new models currently in PRs are using the
@anmarques @bfineran we should probably try to standardize this convention as it seems like we have two variants now, and both seem to be valid? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! clever changes 😀
* Support to Generate Fake Sample/Inputs and outputs if no `--data_args` supplied in export script * Address all review comments Simplify call to `_get_fake_inputs` Save inputs/outputs to `sample-inputs`/`sample-outputs`
This PR adds support to generate fake sample inputs/outputs based on the model shape if no
data_args
are suppliedsupplied in export script. Also fixes the issue #1179
The test model was downloaded as follows:
Before this PR:
An error was raised
After this PR
Test command:
Output: