-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
create helper function #26225
base: master
Are you sure you want to change the base?
create helper function #26225
Conversation
Codecov Report
@@ Coverage Diff @@
## master #26225 +/- ##
==========================================
+ Coverage 71.20% 71.49% +0.28%
==========================================
Files 787 849 +62
Lines 103330 102820 -510
==========================================
- Hits 73581 73507 -74
+ Misses 28252 27816 -436
Partials 1497 1497
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 292 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Assigning reviewers. If you would like to opt out of this review, comment R: @damccorm for label python. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
Run TFT Criteo Benchmarks |
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.
Thanks for doing this! Looks like there are some formatting/linting issues - you can get more detail on them by looking in the logs (linked in the checks section) or by running them locally with the commands described here - https://cwiki.apache.org/confluence/display/BEAM/Python+Tips#PythonTips-LintandFormattingChecks
feature = tf.squeeze(feature, axis=1) | ||
return feature | ||
|
||
fill_in_missing(feature) |
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.
We probably need to reassign these back to feature, right?
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
I tried solving them by referring to the console output but seems like there is still something I'm missing out , I'll look into it again |
Run TFT Criteo Benchmarks |
could you guide me how to do it ? |
Sorry for the confusion, that was a command for our automation (our Jenkins setup will trigger builds based on comments in the PR). So just commenting Seems like we're having some issues with our Jenkins setup, so it may take a while (or I may need to try again later) |
Run TFT Criteo Benchmarks |
Oh, can only reviewers run the tests or the contributors as well? |
Contributors can as well |
Run TFT Criteo Benchmarks |
@damccorm the TFT criteo benchmark test is giving the following error |
Lets run it again, I think it may be fixed by 0dcb26d |
Run TFT Criteo Benchmarks |
1 similar comment
Run TFT Criteo Benchmarks |
this benchmark is failing currently since we updated protobuf to 4.x.x and TFT still relies on protobuf < 4. I am watching TFT pypi for the next release which will make the test suite green |
On that note, I would suggest to do some testing locally for now. |
I tried testing the helper function only with some values and it worked fine |
Reminder, please take a look at this pr: @damccorm |
Unfortunately, we probably should wait until the full workflow can be tested before merging changes. This change seems like its doing roughly the right thing, once TFT is released we should definitely revisit this. Sorry @smeet07, we can leave the PR open until then though |
Stop reviewer notifications |
Stopping reviewer notifications for this pull request: requested by reviewer |
Run TFT Criteo Benchmarks |
@damccorm @AnandInguva The TFT Criteo benchmark test is now passing |
…criteo_test.py Co-authored-by: Anand Inguva <[email protected]>
Create helper function to convert rank 2 tensor to rank 1 tensor
addresses #24902
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123
), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>
instead.CHANGES.md
with noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI.