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

Adding a Resnet 18 Example #268

Merged
merged 22 commits into from
Dec 22, 2023
Merged

Adding a Resnet 18 Example #268

merged 22 commits into from
Dec 22, 2023

Conversation

zjgarvey
Copy link
Collaborator

This pull request adds a simple resnet-18 example with dynamic batches.

@dan-garvey
Copy link
Member

@IanNod or @aviator19941 can one of you TAL at this?

examples/resnet-18/resnet-18.py Outdated Show resolved Hide resolved
@@ -0,0 +1,3 @@
transformers
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe worth considering moving resnet from examples to python/turbine_models in some form to enable use outside turbine?

examples/resnet-18/resnet-18.py Show resolved Hide resolved
x = torch.randn(17, 3, 224, 224)
x[2] = pixel_tensor
y = shark_infer(x)
print_labels(y.to_host())
Copy link
Contributor

Choose a reason for hiding this comment

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

Should add a test for this to ensure we don't break our example models

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 added something like a test, but I'm not sure if it is what you were requesting. I downloaded a bigger dataset and compared the results of the compiled module with the non SHARK-Turbine forward function.

Copy link
Contributor

@IanNod IanNod Dec 19, 2023

Choose a reason for hiding this comment

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

I was more thinking unit tests that will run on the CI so anytime our code is updated it will be checked for any kind of failures on this model. An example of the aot mlp example can be found here for reference https://github.com/nod-ai/SHARK-Turbine/blob/main/tests/examples/aot_mlp_test.py

Copy link
Collaborator Author

@zjgarvey zjgarvey Dec 20, 2023

Choose a reason for hiding this comment

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

Amusingly enough, this example broke today upon updating the pip release of shark_turbine from 9.2 to 9.3.

Copy link
Contributor

Choose a reason for hiding this comment

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

That is unfortunate, my condolences. This does highlight the fact that the tests are important as whatever change broke this example would have failed and would need to be addressed to get merged.

It may be helpful if you can post an issue of what the error is and maybe a minimal reproducer and we can get more eyes on the problem

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will do! Thanks for all the comments so far, @IanNod

Copy link
Contributor

Choose a reason for hiding this comment

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

Will do! Thanks for all the comments so far, @IanNod

Of course, thanks for the work you are doing!

@dan-garvey
Copy link
Member

@zjgarvey can you mark your test as xfail? we can merge this for now if you do

@dan-garvey dan-garvey merged commit 406b523 into nod-ai:main Dec 22, 2023
4 checks passed
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.

3 participants