Write example decompositions as complete functions#709
Conversation
This makes it clearer how the example decompositions map to the methods they are defining - variables are explicitly named as parameters, or unpacked from the arguments. No normative changes.
|
@huningxin & @fdwr - please take a look? |
fdwr
left a comment
fdwr
left a comment
There was a problem hiding this comment.
Thank you for wrapping these all in named functions - that makes it clearer 🧼😎.
|
FYI I'll look into lint tooling to help enforce this, but I didn't want to block on it. The easiest might be pulling eslint in as another node dependency, if that doesn't seem too bad. |
|
Yes, that sets a nice and clear context to the code snippets. |
|
@huningxin take another look, and merge if it looks good? Thanks! |
| const c = builder.clamp(condition, {'minValue': 0, 'maxValue': 1}); | ||
| builder.add( | ||
| builder.mul(input, builder.cast(c, input.dataType())), | ||
| builder.mul(other, builder.cast(builder.logicalNot(c), other.dataType()))); |
There was a problem hiding this comment.
The above two lines were good and won't need to change.
There was a problem hiding this comment.
Oops. :) Thanks.
huningxin
left a comment
huningxin
left a comment
There was a problem hiding this comment.
LGTM, thanks for the fix!
SHA: 588a728 Reason: push, by huningxin Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
4 participants
This makes it clearer how the example decompositions map to the methods they are defining - variables are explicitly named as parameters, or unpacked from the arguments.
Also fix code treating MLOperand's dataType() as a getter instead of a method.
No normative changes.
Preview | Diff