-
Notifications
You must be signed in to change notification settings - Fork 670
FIX-#1953: Fix computing of reduced indices for reduction operation #1960
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
FIX-#1953: Fix computing of reduced indices for reduction operation #1960
Conversation
21b7083 to
dda769e
Compare
Codecov Report
@@ Coverage Diff @@
## master #1960 +/- ##
==========================================
+ Coverage 81.80% 81.88% +0.07%
==========================================
Files 79 79
Lines 9389 9434 +45
==========================================
+ Hits 7681 7725 +44
- Misses 1708 1709 +1
Continue to review full report at Codecov.
|
23d2dfa to
63d599e
Compare
a96eb52 to
985ca0b
Compare
985ca0b to
daccc75
Compare
|
@devin-petersohn , just a friendly reminder to review. |
298f430 to
4e17132
Compare
4e17132 to
18fe443
Compare
|
@dchigarev , your guessing with propagating |
|
The point is that a length of reduced index sometimes can be not equal one. That's why reduced index should be recomputed in some cases. We could propagate recomputing of reduced index (row_lengths and columns_widths as well) into |
|
@YarShev yeah, I think it's a good idea. At |
devin-petersohn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, precomputing row_lengths and column_widths is an optimization. If it is off we should just clear it as you say @YarShev and @dchigarev. The internal system will materialize them when they are needed and they should be correct then.
Was there a lot of code moved around in this PR? It makes the diff a little hard to read.
cddb5e5 to
b817bac
Compare
|
@devin-petersohn , @dchigarev , done as we discussed. |
b817bac to
8c97810
Compare
for reduction operation Signed-off-by: Igoshev, Yaroslav <yaroslav.igoshev@intel.com>
8c97810 to
8fec8a3
Compare
|
@devin-petersohn , just a friendly reminder to review. |
devin-petersohn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @YarShev, LGTM
…ct#1960) for reduction operation Signed-off-by: Igoshev, Yaroslav <yaroslav.igoshev@intel.com>
Signed-off-by: Igoshev, Yaroslav yaroslav.igoshev@intel.com
What do these changes do?
flake8 modinblack --check modingit commit -s