-
Notifications
You must be signed in to change notification settings - Fork 63
sarkars/Upgrade master to 25rc3, bring back failing tests, and add ignore #197
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
Conversation
| command_executor(["pip", "install", "-U", "psutil"]) | ||
|
|
||
| cmd = 'python -m pytest ' + ('--junitxml=%s/xunit_pytest.xml' % build_dir) | ||
| cmd = 'python -m pytest ' + ( |
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.
What's the plan on including this when we need the float tests?
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.
right now on master we have --ignore in run_ngtf_pytests_from_artifacts. to prevent these tests from running (and failing) on CPU ci
This PR adds --ignore to run_ngtf_pytests, which is used in test_ngtf.py. So this just brings run_ngtf_pytests up to speed with run_ngtf_pytests_from_artifacts.
regd plans of removing the --ignore for CI: @shresthamalik might be able to comment on that. For now these tests are used by stakeholders who are interested in them, but we ignore them in CI
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.
The bfloat tests are not valid for all backends. In future we can add a check to get the currently set backend and choose to run them or not run them. These utilities 'run_ngtf_pytests_from_artifacts' and 'run_ngtf_pytests' are mainly used on CPU so it is safe to ignore them.
| git clone https://github.com/tensorflow/ngraph-bridge.git | ||
| cd ngraph-bridge | ||
| git checkout v0.18.0-rc1 | ||
| git checkout v0.18.0-rc2 |
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.
our rc-2 uses ngraph rc3?
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.
yes. we started with ngrc1 (ngrc0 never had a tag from us)
shresthamalik
left a comment
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
| command_executor(["pip", "install", "-U", "psutil"]) | ||
|
|
||
| cmd = 'python -m pytest ' + ('--junitxml=%s/xunit_pytest.xml' % build_dir) | ||
| cmd = 'python -m pytest ' + ( |
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.
The bfloat tests are not valid for all backends. In future we can add a check to get the currently set backend and choose to run them or not run them. These utilities 'run_ngtf_pytests_from_artifacts' and 'run_ngtf_pytests' are mainly used on CPU so it is safe to ignore them.
No description provided.