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

Add tensorflow tensor support #3340

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from

Conversation

darshil929
Copy link

Changes

  • Added support for ord=0 in tf_linalg.py's norm function
  • Fixed the keepdims parameter for nuclear norm case
  • Added tests that check norm with keepdims=True for both CPU and GPU classes
  • Added tests for lstsq function with multiple right-hand sides (rank-2 tensors)

Reason for changes

Right now TensorFlow tensors don't work with nncf.Tensor, which is blocking issue #3041 from being done. This PR adds TensorFlow backend support so we can use TensorFlow tensors with NNCF operations. I'm continuing the work from PR #3106 but fixing some things reviewers pointed out and adding stuff that was missing.

Related tickets

#3041
#3106

Tests

  • Added a test_norm_keepdims function to both CPU and GPU classes to check if keepdims works right
  • Added test_lstsq_rank2 to make sure least squares works with multiple columns
  • Note: I've added the requested tests, but encountered environment setup issues that prevented me from running them successfully. The error appears to be related to function signature registration in the TensorFlow backend integration, which I think is beyond the scope of my changes

@darshil929 darshil929 requested a review from a team as a code owner March 13, 2025 18:06
@github-actions github-actions bot added documentation Improvements or additions to documentation NNCF TF Pull requests that updates NNCF TensorFlow labels Mar 13, 2025
@darshil929 darshil929 marked this pull request as draft March 13, 2025 18:12
@darshil929 darshil929 marked this pull request as ready for review March 13, 2025 18:14
@alexsu52 alexsu52 requested review from kshpv and alexsu52 March 14, 2025 06:50
@github-actions github-actions bot added the API Public API-impacting changes label Mar 14, 2025
@darshil929
Copy link
Author

@kshpv

I've implemented the suggested change to the save_file.register decorator. I've also fixed string literals in exceptions in tf_linalg.py and tf_numeric.py to address the pre-commit linting issues that were causing CI failures. The PR should be ready for review now.

@kshpv
Copy link
Collaborator

kshpv commented Mar 17, 2025

@kshpv

I've implemented the suggested change to the save_file.register decorator. I've also fixed string literals in exceptions in tf_linalg.py and tf_numeric.py to address the pre-commit linting issues that were causing CI failures. The PR should be ready for review now.

Hi @darshil929
Could you please fix precommit issues at first?

…ersion changes, and improve tensor device handling
@darshil929
Copy link
Author

@kshpv
I've implemented the suggested change to the save_file.register decorator. I've also fixed string literals in exceptions in tf_linalg.py and tf_numeric.py to address the pre-commit linting issues that were causing CI failures. The PR should be ready for review now.

Hi @darshil929 Could you please fix precommit issues at first?

I'm analyzing the precommit issues now and will get them fixed as soon as possible.

@darshil929
Copy link
Author

Hi @kshpv

Sir, I've made several changes to fix the type annotations and error handling issues in the backend implementation that were causing the mypy and pytest/tensorflow CI checks to fail. The changes include:

  • Fixed missing return paths in device detection function
  • Added proper type checking for axis parameters in various functions
  • Fixed return type annotation mismatches
  • Added proper type parameters to Callable and ndarray generic types
  • Fixed device mapping type compatibility issues in tensor creation functions
  • Added explicit type handling for expand_dims function to prevent iteration errors

Could you please review these changes and rerun the CI checks to verify they resolve the issues? Please let me know if any additional adjustments are needed.

Thank you!

@alexsu52 alexsu52 requested a review from kshpv March 19, 2025 06:11
@kshpv
Copy link
Collaborator

kshpv commented Mar 19, 2025

Hi @kshpv

Sir, I've made several changes to fix the type annotations and error handling issues in the backend implementation that were causing the mypy and pytest/tensorflow CI checks to fail. The changes include:

  • Fixed missing return paths in device detection function
  • Added proper type checking for axis parameters in various functions
  • Fixed return type annotation mismatches
  • Added proper type parameters to Callable and ndarray generic types
  • Fixed device mapping type compatibility issues in tensor creation functions
  • Added explicit type handling for expand_dims function to prevent iteration errors

Could you please review these changes and rerun the CI checks to verify they resolve the issues? Please let me know if any additional adjustments are needed.

Thank you!

I recommend you run tests locally, it speeds up your development. Please, take a look at failed precommit builds, you need to focus on fixing them and ensure that failed tests pass locally. After that I can make a full-fledged review

@darshil929
Copy link
Author

Hi @kshpv
Sir, I've made several changes to fix the type annotations and error handling issues in the backend implementation that were causing the mypy and pytest/tensorflow CI checks to fail. The changes include:

  • Fixed missing return paths in device detection function
  • Added proper type checking for axis parameters in various functions
  • Fixed return type annotation mismatches
  • Added proper type parameters to Callable and ndarray generic types
  • Fixed device mapping type compatibility issues in tensor creation functions
  • Added explicit type handling for expand_dims function to prevent iteration errors

Could you please review these changes and rerun the CI checks to verify they resolve the issues? Please let me know if any additional adjustments are needed.
Thank you!

I recommend you run tests locally, it speeds up your development. Please, take a look at failed precommit builds, you need to focus on fixing them and ensure that failed tests pass locally. After that I can make a full-fledged review

Yes sir, I understand. I've been experiencing some environment issues with local test execution, but I'll definitely get those resolved first. I'll update you as soon as I have tested everything locally and verified the fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Public API-impacting changes documentation Improvements or additions to documentation NNCF TF Pull requests that updates NNCF TensorFlow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants