-
Notifications
You must be signed in to change notification settings - Fork 46
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
Add MLOperand and update MLOperator description #237
Conversation
363eb65
to
b476887
Compare
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.
Thanks @anssiko . The MLOperand
update looks good to me. Left a minor comment for MLOperator
update, please take a look.
index.bs
Outdated
<script type=idl> | ||
[SecureContext, Exposed=(Window, DedicatedWorker)] | ||
interface MLOperand {}; | ||
</script> | ||
|
||
## MLOperator ## {#api-mloperator} | ||
|
||
The {{MLOperator}} interface defines a type of operation such as the various types of activation function used to create other operations such as [[#api-mlgraphbuilder-conv2d]] or [[#api-mlgraphbuilder-batchnorm]]. | ||
Objects implementing the {{MLOperator}} interface represent activation function types: {{clamp()}}, {{elu()}}, {{hardSigmoid()}}, {{hardSwish()}}, {{leakyRelu()}}, {{linear()}}, {{relu()}}, {{sigmoid()}}, {{softplus()}}, {{softsign()}}, {{tanh()}}. |
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.
Listing the available activation functions would help readers who jump from the operations that support the fused activations, such as conv2d
or batchNormalization
. However, I am not sure whether we should keep mentioning these operations. It might be useful for readers who first read the MLOperator
, so they can jump to conv2d
or batchNormalization
to get an idea of how these activation functions are used.
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.
Thanks for the comment. My idea was to enable navigation from ops supporting fused activations to activation functions. I thought we'd use MLOperator as an intermediary. I'm hearing navigating from MLOperator to supporting ops would also be helpful, and it indeed makes sense to enable cross-linking both ways.
Should we add a complete list of such ops here or is the "such as § 7.7.4 conv2d or § 7.7.1 batchNormalization" good enough?
I pushed an update where the list of activation function becomes an informative note, also noted what type of operations support activations. PTAL.
We could expand that note with a few examples or a complete list of ops that support fused activations.
WDYT?
@huningxin, I pushed an update PTAL. Bikeshed actually has a nice feature that in this case lists activation functions automatically so we don't need to maintain the list manually: |
Thanks @anssiko
It seems to list all references, including all activation functions (e.g., relu) and the operations that use them (e.g., conv2d). Is it possible to distinguish the two categories by bikeshed? |
Not that I know of. Bikeshed parses the IDL for all references to generate this list. |
index.bs
Outdated
The {{MLOperand}} interface represents data that flows within the computational graph. See [[#programming-model]]. | ||
|
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 think calling MLOperand
as the data that flows in the graph is probably a bit misleading. The operand is just a graph construction tool that is logically more similar to a subgraph rather than the data flowing through it. This is because at the time the operand is being used, it represents the graph being constructed up to that point and not the data that flows into it. The only exception to that is when an operand represents a constant.
I would phase it more semantically as something like "An operand represents an intermediary graph being constructed as a result of compositing parts of an operation into a fully composed operation. For instance, an operand may represent a constant feeding to an operation or the result from combining multiple constants together into an operation."
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.
Thanks for this detailed description. That high-level description was borrowed from the programming model section. I updated this PR per your suggestion. Given the programming model links to this description, I think it is fine to have the proper description here with the interface definition.
index.bs
Outdated
<script type=idl> | ||
[SecureContext, Exposed=(Window, DedicatedWorker)] | ||
interface MLOperand {}; | ||
</script> | ||
|
||
## MLOperator ## {#api-mloperator} | ||
|
||
The {{MLOperator}} interface defines a type of operation such as the various types of activation function used to create other operations such as [[#api-mlgraphbuilder-conv2d]] or [[#api-mlgraphbuilder-batchnorm]]. | ||
Objects implementing the {{MLOperator}} interface represent activation function types. |
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 think we might want to leave some room for the future here and not prematurely limiting the notion of an MLOperator
, which is a generic notion, to just the activation functions. It could also be confusing when the notion of the activation function appears twice in the API in different context, one concretely represented by MLOperator
and another as standalone operations off the MLGraphBuilder
.
The idea behind MLOperator
is to allow deferred definition of activation functions used during the construction of a fused operation e.g. GRU, etc. while allowing for future extension without changing the fused operation's API signature.
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.
Mentioning the extensibility for future-proofing is a good idea, PR updated.
Would it make any sense to specialize MLActivation : MLOperator
?
index.bs
Outdated
|
||
<script type=idl> | ||
[SecureContext, Exposed=(Window, DedicatedWorker)] | ||
interface MLOperator {}; | ||
</script> | ||
|
||
<div class="note"> | ||
The implementation of this interface can simply be a struct that holds a string type of the operation along with other properties needed. The actual creation of the operation e.g. a [[#api-mlgraphbuilder-sigmoid]] or [[#api-mlgraphbuilder-relu]] can then be deferred until when the rest of the graph is ready to connect with it such as during the construction of [[#api-mlgraphbuilder-conv2d]] for example. | ||
These activations function types are used to create other operations such as [[#api-mlgraphbuilder-conv2d]] or [[#api-mlgraphbuilder-batchnorm]]. |
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.
We can say something like "one such use of this interface is for when an activation function is fused into another operation such as convolution or batchnorm during a graph construction session.."
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.
Thanks, updated per your suggestion.
Bikeshed generates the list automatically.
4b15fd5
to
b08444a
Compare
PR updated, PTAL @wchao1115 |
@wchao1115 thanks! Please merge at will coordinating with your and @huningxin's other work to avoid merge conflicts. |
With approvals, let's merge this PR and rebase others on top of this. |
…desc SHA: 19a09ab Reason: push, by @huningxin Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
I started filling in a high-level description for the MLOperand that was missing referring to the programming model section for a more elaborate description. As a drive-by, I reworked the MLOperator description a bit to refer to the activation functions explicitly. Perhaps having this list here helps the readers? I hope it is not too hard to maintain if we add more activation functions.
Preview | Diff