Bit-wise AND OR XOR #366
Bit-wise AND OR XOR #366
Conversation
| @@ -11,6 +11,127 @@ | |||
|
|
|||
| __all__ = [] | |||
|
|
|||
| def __binary_bit_op(method, t1, t2): | |||
There was a problem hiding this comment.
I am not 100% sure it is necessary to have this as a separate function in contrast to __binary_op(). It also seems that it is almost a verbatim copy of the function below. Why would we needs this except the type checking and casting purposes? It would be more meaningful from a SE perspective to factor out the common parts from __binary_op and __bitwise_binary_op
There was a problem hiding this comment.
You're right. It is mostly a copy of the function and I am reluctant because of that, too. However, I couldn't find a really satisfying solution at the moment.
What's different is the not desired type casting at the end of each of the if branches at the first half of the function. It still may be not be the best solution, but I'll try to factor each branch out for instance, scalar and mixed pairs to reduce some code redundancy.
Also, the function calls at the end uses the overloaded operators of the torch.tensor class. I don't think I can do much there.
heat/core/operations.py
Outdated
| elif t2.split is None: | ||
| t2 = factories.array(t2, split=t1.split, copy=False, comm=t2.comm, device=t2.device, ndmin=-t1.numdims) | ||
| elif t1.split != t2.split: | ||
| # It is NOT possible to perform binary operations on tensors with different splits, e.g. split=0 |
There was a problem hiding this comment.
Have an issue documenting the fact that we need to do something here still as soon as resplit is implemented (soon done)
Codecov Report
@@ Coverage Diff @@
## master #366 +/- ##
==========================================
+ Coverage 97.41% 97.48% +0.07%
==========================================
Files 53 53
Lines 10586 10793 +207
==========================================
+ Hits 10312 10522 +210
+ Misses 274 271 -3
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #366 +/- ##
==========================================
- Coverage 97.1% 96.93% -0.17%
==========================================
Files 53 53
Lines 9468 9508 +40
==========================================
+ Hits 9194 9217 +23
- Misses 274 291 +17
Continue to review full report at Codecov.
|
|
Minor remark for the future. Please have you branchname of the form: feature/<issue_number>-short-description No worries right now. Will make it easier for us in the future to maybe get statistics across the branches. Thanks |
|
So what is the status on this one? Do you think there is a way to refactor the respective methods or not? |
|
I rearranged it. The separate branches are put in respective functions now. |
Description
Fixes: #226
Changes proposed:
Type of change
The added functions call the new binary_bit_op function giving the method name.
Quite similar to binary_op, but calls the respective operator in torch.array and checks for same integer or boolean types in both tensors.
Please delete options that are not relevant.
Have you handled and tested all split configurations?
No