tensorflow icon indicating copy to clipboard operation
tensorflow copied to clipboard
Published 1 week ago • verified publisher icon tensorflow

Fix strided_slice legalization when input_dim % stride != 0

Open jamwar01 opened this issue 2 years ago • 9 comments

  • Add padding to round an input dim up to the nearest multiple of the corresponding stride value

Change-Id: I1b9677c80e7ba704897637a3c44545d24cdae892

jamwar01 avatar Oct 02 '23 17:10 jamwar01

Hi @rsuderman, Can you please review this PR ? Thank you!

gbaned avatar Nov 03 '23 09:11 gbaned

Hi @jpienaar, Can you please review this PR ? Thank you!

gbaned avatar Dec 29 '23 08:12 gbaned

Hi @rdzhabarov, Can you please review this PR ? Thank you!

gbaned avatar Mar 08 '24 16:03 gbaned

Hi @jpienaar, Can you please review this PR ? Thank you!

gbaned avatar Apr 26 '24 08:04 gbaned

Hi @jpienaar, Can you please review this PR ? Thank you!

gbaned avatar Jun 07 '24 16:06 gbaned

Hi @jpienaar, Can you please review this PR ? Thank you!

keerthanakadiri avatar Jul 09 '24 07:07 keerthanakadiri

Hi @jpienaar, Can you please review this PR ? Thank you!

keerthanakadiri avatar Jul 17 '24 05:07 keerthanakadiri

Hi @jpienaar, Can you please review this PR ? Thank you!

keerthanakadiri avatar Aug 07 '24 09:08 keerthanakadiri

Hi @jpienaar, Can you please review this PR ? Thank you!

keerthanakadiri avatar Aug 28 '24 07:08 keerthanakadiri

Hi @jpienaar, Can you please review this PR ? Thank you!

keerthanakadiri avatar Sep 03 '24 07:09 keerthanakadiri

Hi @jpienaar, Can you please review this PR ? Thank you!

keerthanakadiri avatar Sep 18 '24 04:09 keerthanakadiri

Hi @jpienaar, Can you please review this PR ? Thank you!

keerthanakadiri avatar Oct 10 '24 08:10 keerthanakadiri

@jamwar01 can you rebase this patch to solve the conflict, so that we can proceed with review and merge? Thanks

leandron avatar Feb 07 '25 10:02 leandron

Am I correct in thinking that this patch solves the same problem as another open patch, https://github.com/tensorflow/tensorflow/pull/61424?

ekalda avatar Feb 13 '25 11:02 ekalda

@jamwar01 can you have a look on @ekalda's question and update the PR to solve conflicts if it is still the case? Thanks.

leandron avatar Mar 10 '25 14:03 leandron

Hey @ekalda, yes it does look like both patches address the same issue -- good catch! Let me know how you would like to proceed.

jamwar01 avatar Mar 21 '25 17:03 jamwar01

Hi @jamwar01, thanks for looking again into the issue! I did some git archeology and it seems like the main logic of this patch was added with https://github.com/tensorflow/tensorflow/commit/0f1388eee0b7ceb3660bc9a0790ab2040538af06 (I'm not sure why) and the tests were added in https://github.com/tensorflow/tensorflow/pull/88530 just few weeks ago. So I think this PR and the duplicate can both be closed.

ekalda avatar Mar 25 '25 15:03 ekalda

Ok thanks for digging into this!

jamwar01 avatar Mar 26 '25 13:03 jamwar01