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

continue-on-error: true for mac tvm install fail #728

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ksaur
Copy link
Collaborator

@ksaur ksaur commented Jul 19, 2023

For #727

@codecov-commenter
Copy link

Codecov Report

Merging #728 (9378a89) into main (166f11e) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #728   +/-   ##
=======================================
  Coverage   90.20%   90.20%           
=======================================
  Files          80       80           
  Lines        4676     4676           
  Branches      856      856           
=======================================
  Hits         4218     4218           
  Misses        260      260           
  Partials      198      198           
Flag Coverage Δ
unittests 90.20% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
hummingbird/__init__.py 100.00% <100.00%> (ø)

@ksaur
Copy link
Collaborator Author

ksaur commented Jul 19, 2023

This does work, but now it fails silently. (The pipeline continues, runs all non-TVM tests on mac, and gives green checkmark without showing any warnings/etc.) This seems not a perfect solution either. There's probably some cleaner way to get alerts on this while not blocking everything.

Thoughts Matteo/Masahiro?

Screenshot 2023-07-19 at 11 07 57 AM

@ksaur ksaur requested review from mshr-h and interesaaat and removed request for mshr-h July 19, 2023 18:08
Copy link
Collaborator

@mshr-h mshr-h left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@interesaaat
Copy link
Collaborator

Is this a draft or good to go?

@ksaur ksaur marked this pull request as ready for review July 20, 2023 15:51
@ksaur
Copy link
Collaborator Author

ksaur commented Jul 20, 2023

I just wanted both of your inputs to see if this is behavior we wanted. It may make things easier in future, but I also want to make sure mac TVM is supported when/where possible!

@ksaur ksaur marked this pull request as draft July 20, 2023 15:58
@ksaur
Copy link
Collaborator Author

ksaur commented Jul 20, 2023

Leaving this as a draft....ready to go if needed later (hopefully not). :)

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.

None yet

4 participants