GeezerVison

Codecov Report

Merging #501 (baf4709) into main (7935e50) will increase coverage by 0.19%.
The diff coverage is 99.69%.

@@            Coverage Diff             @@
##             main     #501      +/-   ##
==========================================
+ Coverage   99.20%   99.39%   +0.19%     
==========================================
  Files          95       95              
  Lines       21291    21884     +593     
  Branches     3999     4138     +139     
==========================================
+ Hits        21121    21751     +630     
+ Misses        124       75      -49     
- Partials       46       58      +12     

Hey @jim22k @SultanOrazbayev @michelp, this is pretty much ready if you want to take a look--I just need to write the error message when mixing infix notations such as binop(x & y | z).

This allows operators for ewise add, ewise mult, and matrix multiplication to apply to all the operands inside the operator when using infix notation such as plus(a | b | c | d) or plus_plus(A @ B @ C). This doesn't do any fancy reordering, applying of masks, or reuse of intermediates--it simply automatically computes the intermediate values (regardless of gb.config["autocompute"] setting).

Mixing ewise infix notation now raises: binop(x & y | z) is not allowed. This is because I think many people get tripped up by the operator precedence of & and |. The only time this may be a nuisance is if you are dealing with boolean values and are comfortable using & and | together, but I think it's better for the sake of future readers to require explicit lor and land as appropriate.

Applying operators like this also only apply to infix notation. plus(a | b | c) is not the same as a.ewise_add(b | c, plus) or (a | b).ewise_add(c, plus) (both of which are kind of weird and will probably raise).

Overall I think this change is for the better. I think the main "gotcha" this introduces is this:

>>> C = A @ B
>>> E << min_plus(C @ D)

Did they mean C = min_plus(A @ B) or C = plus_times(A @ B)? This could be avoided many different ways (depending on what is intended):

1. C = (A @ B).new()
2. C << A @ B
3. C = semiring(A @ B)
4. C << semiring(A @ B)
5. AatB = A @ B # Using a better name to indicate an operator may still be applied

Hence, it's best practice to either be explicit with a semiring, call .new(), or assign into an output via <<.

Unless we want to detect whether an expression is assigned or inlined (which is possible, but let's not introduce that voodoo magic if we don't absolutely need to), this is the tradeoff we are stuck with. I think the multi-infix notation can be used well, as @alugowski did in #498. Edge cases exist here no matter what we do (I think), and I'm inclined to support what multiple users expected to work, which is what this PR does. This PR goes further by detecting and disallowing other syntax that is likely to be incorrect.

When we discussed this possibility before in #132, @ParticularMiner liked applying the operator to all infix operands as done in this PR (see: #132 (comment))

This PR should now be easier to review b/c I added unrelated changes into main via #517.

I'm okay with this approach. I agree that gb.min_plus(A @ B @ C) is more likely than X = A @ B; gb.min_plus(X @ C) and there will be some confusion either way. Such is a nature of delayed computation systems.

This is in--thanks all!

I found it nice to use this syntax when creating #518 (I wrote semiring(I @ A @ I))