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

Update Gather-7 specification #5441

Conversation

achetver
Copy link
Contributor

@achetver achetver commented Apr 28, 2021

Details:

  • Update Gather-7 operation specification since we decide to support negative values of batch_dims attribute. Also update formula to same view as in TF specification to make it more clear.

Tickets:

  • 54329

@achetver achetver requested a review from a team as a code owner April 28, 2021 10:42
@openvino-pushbot openvino-pushbot added the category: docs OpenVINO documentation label Apr 28, 2021
@achetver achetver changed the title Doc/achetver/update gather 7 doc Update Gather-7 specification Apr 28, 2021
@achetver achetver requested review from a team, iimironov, sadolini and yekruglov and removed request for a team April 29, 2021 07:31
@achetver achetver added this to the 2021.4 milestone Apr 29, 2021
@achetver
Copy link
Contributor Author

@sadolini @yekruglov @iimironov @lazarevevgeny Please, review this PR with important changes in Gather-7 operation specification.

@a-sidorova
Copy link
Contributor

I think that description for axis is incorrect:
Allowed values are from [-len(data.shape), len(indices.shape) - 1] and axis >= batch_dims

My suggestion is:
Allowed values are from [-len(data.shape), len(data.shape) - 1] and axis >= batch_dims

Because otherwise the example 4 contradicts this description. axis = 2 but len(indices.shape) - 1 = 2 - 1 =1.

@andrei-cv
Copy link
Contributor

Allowed values are from [-len(indices.shape), len(indices.shape) - 1] and axis >= batch_dims as in tensorflow

docs/ops/movement/Gather_7.md Outdated Show resolved Hide resolved
docs/ops/movement/Gather_7.md Outdated Show resolved Hide resolved
@pavel-esir pavel-esir requested a review from sadolini May 5, 2021 07:42
@pavel-esir
Copy link
Contributor

pavel-esir commented May 5, 2021

Allowed values are from [-len(indices.shape), len(indices.shape) - 1] and axis >= batch_dims as in tensorflow

@andrei-cv indeed there must be data.shape not indices. Axis value is not bound to indices. In TF for axis allowed values are
Allowed values are from [-len(data.shape), len(data.shape) - 1] and axis >= batch_dims

https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/kernels/gather_op.cc#L81-L89
there params goes for 1st input in our case means data

@pavel-esir pavel-esir requested a review from lazarevevgeny May 5, 2021 09:30
@lazarevevgeny lazarevevgeny merged commit e86b9b1 into openvinotoolkit:master May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: docs OpenVINO documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants