feat: support uint8 and int8 allreduce in tensorflow

[kvignesh1420]

Checklist before submitting

• Did you read the contributor guide?
• Did you update the docs?
• Did you write any tests to validate this change?
• Did you update the CHANGELOG, if this change affects users?

Description

This PR adds support for int8 and uint8 allreduce in tensorflow along with the relevant tests.
Corresponding issue: #3642

Signed-off-by: Vignesh Kothapalli k.vignesh1420@gmail.com

Review process to land

1. All tests and other checks must succeed.
2. At least one member of the technical steering committee must review and approve.
3. If any member of the technical steering committee requests changes, they must be addressed.

[maxhgerlach]

Thanks for taking a stab at this, @kvignesh1420!

I've triggered the CI pipeline and have left one comment regarding the test script.

[github-actions]

Unit Test Results

  1 193 files   -   38    1 193 suites   - 38   11h 41m 28s ⏱️ - 17m 32s
     816 tests ±    0       765 ✔️ ±    0       51 💤 ±    0  0 ❌ ±0 
23 443 runs   - 816  16 691 ✔️  - 506  6 752 💤  - 310  0 ❌ ±0 

Results for commit a02bddc. ± Comparison against base commit 867741e.

♻️ This comment has been updated with latest results.

[github-actions]

Unit Test Results (with flaky tests)

  1 371 files   -   63    1 371 suites   - 63   12h 31m 31s ⏱️ - 26m 1s
     816 tests ±    0       765 ✔️ +    2       51 💤 ±    0  0 ❌  - 2 
26 889 runs   - 926  18 645 ✔️  - 528  8 244 💤  - 396  0 ❌  - 2 

Results for commit a02bddc. ± Comparison against base commit 867741e.

♻️ This comment has been updated with latest results.

[kvignesh1420]

@maxhgerlach I reverted the sampling range back to (-100, 100) and fixed the tests for int8, uint8 when average=True. Please take a look.

[maxhgerlach]

Thank you @kvignesh1420, that looks great!

There is a bunch of additional TensorFlow allreduce tests, scattered over a couple of separate files:

• test_tensorflow_process_sets.py
• test_xla.py
• test_xla_process_sets.py

I think it would be good to extend the (u)int8 test coverage at least to test_xla.py as that one uses a different backend than test_tensorflow.py.

There would be less to gain from the various tests with multiple process sets in the two other files, so I'm not sure if those are worth the busy work.

[kvignesh1420]

Thank you @kvignesh1420, that looks great!

There is a bunch of additional TensorFlow allreduce tests, scattered over a couple of separate files:

• test_tensorflow_process_sets.py
• test_xla.py
• test_xla_process_sets.py

I think it would be good to extend the (u)int8 test coverage at least to test_xla.py as that one uses a different backend than test_tensorflow.py.

There would be less to gain from the various tests with multiple process sets in the two other files, so I'm not sure if those are worth the busy work.

@maxhgerlach I modified the tests in those 3 files as well. Please let me know if this looks good 🙂 .

[maxhgerlach]

LGTM, let's just wait for the tests to pass. Thank you for the contribution!

[maxhgerlach]

Tests are all good. The failure at "CI / Build docker image horovod-ray" is unrelated.