create helper function #26225

smeet07 · 2023-04-11T19:54:45Z

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:

Mention the appropriate issue in your description (for example: 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, comment fixes #<ISSUE NUMBER> instead.
Update CHANGES.md with noteworthy changes.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

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.

Create helper function to convert rank 2 tensor to rank 1 tensor

codecov · 2023-04-11T20:13:49Z

Codecov Report

Merging #26225 (f5b25b5) into master (d3f1acb) will increase coverage by 0.28%.
The diff coverage is 0.00%.

@@            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

Flag	Coverage Δ
python	`80.32% <0.00%> (+0.48%)`	⬆️

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

Files Changed	Coverage Δ
...am/testing/benchmarks/cloudml/criteo_tft/criteo.py	`0.00% <0.00%> (ø)`

... and 292 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

github-actions · 2023-04-11T22:34:33Z

Assigning reviewers. If you would like to opt out of this review, comment assign to next reviewer:

R: @damccorm for label python.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

damccorm · 2023-04-12T15:57:52Z

Run TFT Criteo Benchmarks

damccorm

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

damccorm · 2023-04-12T16:00:24Z

sdks/python/apache_beam/testing/benchmarks/cloudml/criteo_tft/criteo.py

+        feature = tf.squeeze(feature, axis=1)
+        return feature
+
+      fill_in_missing(feature)


We probably need to reassign these back to feature, right?

smeet07 · 2023-04-12T16:06:50Z

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

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

damccorm · 2023-04-12T19:29:09Z

Run TFT Criteo Benchmarks

smeet07 · 2023-04-12T19:49:51Z

Run TFT Criteo Benchmarks

could you guide me how to do it ?

damccorm · 2023-04-12T20:04:19Z

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 Run TFT Criteo Benchmarks should eventually cause it to run. I'll comment again so it triggers against the most recent commit

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)

damccorm · 2023-04-12T20:04:25Z

Run TFT Criteo Benchmarks

smeet07 · 2023-04-12T20:13:40Z

Oh, can only reviewers run the tests or the contributors as well?

damccorm · 2023-04-12T20:15:56Z

Contributors can as well

damccorm · 2023-04-13T12:59:14Z

Run TFT Criteo Benchmarks

smeet07 · 2023-04-13T15:15:40Z

@damccorm the TFT criteo benchmark test is giving the following error
ImportError: cannot import name 'builder' from 'google.protobuf.internal' (/home/jenkins/jenkins-slave/workspace/beam_CloudML_Benchmarks_Dataflow_PR/src/build/gradleenv/-1734967050/lib/python3.9/site-packages/google/protobuf/internal/init.py)

damccorm · 2023-04-14T17:16:13Z

Lets run it again, I think it may be fixed by 0dcb26d

damccorm · 2023-04-14T17:16:24Z

Run TFT Criteo Benchmarks

smeet07 · 2023-04-14T19:04:10Z

Run TFT Criteo Benchmarks

AnandInguva · 2023-04-14T19:30:30Z

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

AnandInguva · 2023-04-14T19:30:55Z

On that note, I would suggest to do some testing locally for now.

smeet07 · 2023-07-08T13:21:12Z

Run PythonLint PreCommit

smeet07 · 2023-07-08T13:59:52Z

Run PythonLint PreCommit

smeet07 · 2023-07-12T17:58:12Z

@AnandInguva
I was getting apache_beam/testing/benchmarks/cloudml/criteo_tft/criteo_test.py:30: error: Incompatible types in assignment (expression has type "None", variable has type "Callable[[Any, Any], Any]") [assignment] error so I changed the import statement of fill in missing to account for None as
fill_in_missing: Optional[Callable[[tf.sparse.SparseTensor, int], tf.Tensor]] = None .
But now I am getting AttributeError: 'NoneType' object has no attribute 'sparse' because tf is not getting imported correctly and is present in the same try block. can you suggest some workaround for this or the original error?

AnandInguva · 2023-07-14T14:15:30Z

@AnandInguva I was getting apache_beam/testing/benchmarks/cloudml/criteo_tft/criteo_test.py:30: error: Incompatible types in assignment (expression has type "None", variable has type "Callable[[Any, Any], Any]") [assignment] error so I changed the import statement of fill in missing to account for None as fill_in_missing: Optional[Callable[[tf.sparse.SparseTensor, int], tf.Tensor]] = None . But now I am getting AttributeError: 'NoneType' object has no attribute 'sparse' because tf is not getting imported correctly and is present in the same try block. can you suggest some workaround for this or the original error?

Thanks. I will take a look in some time.

sdks/python/apache_beam/testing/benchmarks/cloudml/criteo_tft/criteo_test.py

AnandInguva · 2023-07-18T12:43:23Z

sdks/python/apache_beam/testing/benchmarks/cloudml/criteo_tft/criteo.py

+try:
+  import tensorflow as tf
+  import tensorflow_transform as tft
+except ImportError as e:


you don't need this try except here. It is needed only in the tests.

Not adding it was causing import error that is why added it , I'll remove it.

You should import criteo.py in tests under a try except block since criteo.py has a dependency on tft which might not be installed on all test environments.

Yeah I've imported criteo.py in a try block

AnandInguva · 2023-07-18T12:43:33Z

sdks/python/apache_beam/testing/benchmarks/cloudml/criteo_tft/criteo.py

@@ -110,6 +113,20 @@ def make_input_feature_spec(include_label=True):
  return result


+def fill_in_missing(feature, default_value=-1):
+  if tf is not None:


remove this if condition as well.

…criteo_test.py Co-authored-by: Anand Inguva <[email protected]>

AnandInguva · 2023-07-18T15:52:24Z

sdks/python/apache_beam/testing/benchmarks/cloudml/criteo_tft/criteo_test.py

+  import tensorflow as tf
+  from apache_beam.testing.benchmarks.cloudml.criteo_tft.criteo import fill_in_missing
+except ImportError:
+  tft = None  # type: ignore[assignment]


My bad. Remove the # type: ignore[assignment].

Mypy check should pass after that

AnandInguva · 2023-07-18T15:53:29Z

sdks/python/apache_beam/testing/benchmarks/cloudml/criteo_tft/criteo_test.py

+import numpy as np
+import pytest
+
+from typing import Any, Callable, Optional


Remove Any, Callable from imports since they are not being used anywhere.

Lint errors would get solved after this.

smeet07 · 2023-07-25T04:56:24Z

@AnandInguva I'm having trouble finding out the exact cause of github workflow checks failing since the console output is too large, what do you look for while browsing through the files?

AnandInguva · 2023-07-25T05:26:28Z

The Jenkins tests are passing but GHA tests are not. Can you merge master branch on to yours?

AnandInguva · 2023-07-25T07:13:08Z

Run TFT Criteo Benchmarks

smeet07 · 2023-09-28T17:03:49Z

@AnandInguva I'm not able to merge it on my this computer as git clone is not cloning the whole repo

AnandInguva · 2023-09-28T17:22:58Z

able

Sorry. I didn't get what you are saying. Can you provide which commands you are using?

smeet07 · 2023-10-02T07:26:31Z

able

Sorry. I didn't get what you are saying. Can you provide which commands you are using?

git clone is not copying the whole code for some reason in my new laptop

damccorm · 2023-10-27T15:01:18Z

@smeet07 any luck getting this fixed up? Sounds like your local git config is currently broken, my recommendation would probably be to try uninstalling/reinstalling.

You could also try opening this in a GitHub codespace - https://github.com/features/codespaces

smeet07 · 2023-10-29T11:40:32Z

forgot about this issue tbh, will get this fixed up in 1-2 days

liferoad · 2024-04-11T13:28:29Z

@smeet07 are you still working on this?

create helper function

c1eb39d

Create helper function to convert rank 2 tensor to rank 1 tensor

github-actions bot added the python label Apr 11, 2023

smeet07 added 2 commits April 12, 2023 02:34

formatting

333349e

formatting again

a6805e4

github-actions bot added the Next Action: Reviewers label Apr 11, 2023

damccorm reviewed Apr 12, 2023

View reviewed changes

smeet07 added 2 commits April 12, 2023 21:41

re assigning it back to the feature

5886181

trailing whitespace

3d0ded9

smeet07 added 2 commits April 13, 2023 01:30

whitespace changes

b22e3a7

changes

bbe227a

smeet07 added 2 commits April 13, 2023 02:30

replacing whitespace by tabs

b28d624

Update criteo.py

5b04990

sparse_to_dense syntax has been changed in tf2.0

a4dddc9

smeet07 added 7 commits July 5, 2023 21:13

fix import issue

e5238ec

indentation

3496d59

spacing issues

8ca9a5d

lint issues

9268b74

add space

ff25761

add

e186f89

fix call

1af5bf8

Update criteo_test.py

e65c46a

smeet07 added 2 commits July 9, 2023 10:50

Update criteo_test.py

cf828d5

Update criteo_test.py

e8756c0

AnandInguva reviewed Jul 18, 2023

View reviewed changes

smeet07 and others added 3 commits July 18, 2023 18:55

Update sdks/python/apache_beam/testing/benchmarks/cloudml/criteo_tft/…

8f9bbe9

…criteo_test.py Co-authored-by: Anand Inguva <[email protected]>

remove try block

596f424

remove double except block

8216b0d

AnandInguva reviewed Jul 18, 2023

View reviewed changes

smeet07 added 2 commits July 18, 2023 22:27

remove imports and type assignment

b595fcb

remove trailing whitespace

f5b25b5

create helper function #26225

Are you sure you want to change the base?

create helper function #26225

Conversation

smeet07 commented Apr 11, 2023 • edited Loading

GitHub Actions Tests Status (on master branch)

codecov bot commented Apr 11, 2023 • edited Loading

Codecov Report

github-actions bot commented Apr 11, 2023

damccorm commented Apr 12, 2023

damccorm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smeet07 commented Apr 12, 2023

damccorm commented Apr 12, 2023

smeet07 commented Apr 12, 2023

damccorm commented Apr 12, 2023 • edited Loading

damccorm commented Apr 12, 2023

smeet07 commented Apr 12, 2023

damccorm commented Apr 12, 2023

damccorm commented Apr 13, 2023

smeet07 commented Apr 13, 2023

damccorm commented Apr 14, 2023

damccorm commented Apr 14, 2023

smeet07 commented Apr 14, 2023

AnandInguva commented Apr 14, 2023

AnandInguva commented Apr 14, 2023

smeet07 commented Jul 8, 2023

smeet07 commented Jul 8, 2023

smeet07 commented Jul 12, 2023

AnandInguva commented Jul 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smeet07 commented Jul 25, 2023

AnandInguva commented Jul 25, 2023

AnandInguva commented Jul 25, 2023

smeet07 commented Sep 28, 2023

AnandInguva commented Sep 28, 2023

smeet07 commented Oct 2, 2023

damccorm commented Oct 27, 2023

smeet07 commented Oct 29, 2023

liferoad commented Apr 11, 2024

smeet07 commented Apr 11, 2023 •

edited

Loading

codecov bot commented Apr 11, 2023 •

edited

Loading

damccorm commented Apr 12, 2023 •

edited

Loading