Skip to content

Commit

Permalink
Return errors as records, not resources
Browse files Browse the repository at this point in the history
This change migrates the `error` type to return as a record instead of a
resource. The previous logic behind #64, IIRC, is that users that may
not wish to copy the potentially-large `data` string across the
canonical ABI would not have to--they could call the `data` method on
the resource if they really needed it.

Technical complexity during implementation is making me reconsider this
decision. While implementing this in Wasmtime, I realized that after an
error happens, the current "error as resource" scheme results in a
multi-step failure algorithm:
- construct the internal host error
- register the host error in the resource table--this may fail!
- return the host error as a resource

This complexity does not mean necessarily that the spec _must_ be
changed; I believe there is a way to "make it work." But if users end up
troubleshooting a resource failure that happens during a wasi-nn
failure, we risk making things a bit too complex for them. And, if
indeed the original argument for using resources was avoiding
performance overhead, there is a case to be made that in failure modes
performance is not the most critical for users.

I'll open this seeking feedback, not to merge immediately.
  • Loading branch information
abrown committed Jul 31, 2024
1 parent 917bf47 commit 22e4af8
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 44 deletions.
54 changes: 17 additions & 37 deletions ml.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,38 +135,18 @@ e.g., cannot access a hardware feature requested
<p>The operation failed for an unspecified reason.
</li>
</ul>
<h4><a name="error"></a><code>resource error</code></h4>
<hr />
<h3>Functions</h3>
<h4><a name="constructor_error"></a><code>[constructor]error: func</code></h4>
<h5>Params</h5>
<ul>
<li><a name="constructor_error.code"></a><code>code</code>: <a href="#error_code"><a href="#error_code"><code>error-code</code></a></a></li>
<li><a name="constructor_error.data"></a><code>data</code>: <code>string</code></li>
</ul>
<h5>Return values</h5>
<ul>
<li><a name="constructor_error.0"></a> own&lt;<a href="#error"><a href="#error"><code>error</code></a></a>&gt;</li>
</ul>
<h4><a name="method_error_code"></a><code>[method]error.code: func</code></h4>
<p>Return the error code.</p>
<h5>Params</h5>
<ul>
<li><a name="method_error_code.self"></a><code>self</code>: borrow&lt;<a href="#error"><a href="#error"><code>error</code></a></a>&gt;</li>
</ul>
<h5>Return values</h5>
<ul>
<li><a name="method_error_code.0"></a> <a href="#error_code"><a href="#error_code"><code>error-code</code></a></a></li>
</ul>
<h4><a name="method_error_data"></a><code>[method]error.data: func</code></h4>
<p>Errors can propagated with backend specific status through a string value.</p>
<h5>Params</h5>
<h4><a name="error"></a><code>record error</code></h4>
<h5>Record Fields</h5>
<ul>
<li><a name="method_error_data.self"></a><code>self</code>: borrow&lt;<a href="#error"><a href="#error"><code>error</code></a></a>&gt;</li>
</ul>
<h5>Return values</h5>
<ul>
<li><a name="method_error_data.0"></a> <code>string</code></li>
<li>
<p><a name="error.code"></a><code>code</code>: <a href="#error_code"><a href="#error_code"><code>error-code</code></a></a></p>
<p>The error code identifies the general reason the operation failed.
</li>
<li>
<p><a name="error.data"></a><code>data</code>: <code>string</code></p>
<p>The data field propagates the backend failure context as a string for easier
troubleshooting.
</li>
</ul>
<h2><a name="wasi_nn_inference_0_2_0_rc_2024_06_25"></a>Import interface wasi:nn/[email protected]</h2>
<p>An inference &quot;session&quot; is encapsulated by a <a href="#graph_execution_context"><code>graph-execution-context</code></a>. This structure binds a
Expand Down Expand Up @@ -197,7 +177,7 @@ e.g., cannot access a hardware feature requested
</ul>
<h5>Return values</h5>
<ul>
<li><a name="method_graph_execution_context_set_input.0"></a> result&lt;_, own&lt;<a href="#error"><a href="#error"><code>error</code></a></a>&gt;&gt;</li>
<li><a name="method_graph_execution_context_set_input.0"></a> result&lt;_, <a href="#error"><a href="#error"><code>error</code></a></a>&gt;</li>
</ul>
<h4><a name="method_graph_execution_context_compute"></a><code>[method]graph-execution-context.compute: func</code></h4>
<p>Compute the inference on the given inputs.</p>
Expand All @@ -210,7 +190,7 @@ https://github.com/WebAssembly/wasi-nn/issues/43.</p>
</ul>
<h5>Return values</h5>
<ul>
<li><a name="method_graph_execution_context_compute.0"></a> result&lt;_, own&lt;<a href="#error"><a href="#error"><code>error</code></a></a>&gt;&gt;</li>
<li><a name="method_graph_execution_context_compute.0"></a> result&lt;_, <a href="#error"><a href="#error"><code>error</code></a></a>&gt;</li>
</ul>
<h4><a name="method_graph_execution_context_get_output"></a><code>[method]graph-execution-context.get-output: func</code></h4>
<p>Extract the outputs after inference.</p>
Expand All @@ -221,7 +201,7 @@ https://github.com/WebAssembly/wasi-nn/issues/43.</p>
</ul>
<h5>Return values</h5>
<ul>
<li><a name="method_graph_execution_context_get_output.0"></a> result&lt;own&lt;<a href="#tensor"><a href="#tensor"><code>tensor</code></a></a>&gt;, own&lt;<a href="#error"><a href="#error"><code>error</code></a></a>&gt;&gt;</li>
<li><a name="method_graph_execution_context_get_output.0"></a> result&lt;own&lt;<a href="#tensor"><a href="#tensor"><code>tensor</code></a></a>&gt;, <a href="#error"><a href="#error"><code>error</code></a></a>&gt;</li>
</ul>
<h2><a name="wasi_nn_graph_0_2_0_rc_2024_06_25"></a>Import interface wasi:nn/[email protected]</h2>
<p>A <a href="#graph"><code>graph</code></a> is a loaded instance of a specific ML model (e.g., MobileNet) for a specific ML
Expand Down Expand Up @@ -274,7 +254,7 @@ graph IR in parts (e.g., OpenVINO stores its IR and weights separately).</p>
</ul>
<h5>Return values</h5>
<ul>
<li><a name="method_graph_init_execution_context.0"></a> result&lt;own&lt;<a href="#graph_execution_context"><a href="#graph_execution_context"><code>graph-execution-context</code></a></a>&gt;, own&lt;<a href="#error"><a href="#error"><code>error</code></a></a>&gt;&gt;</li>
<li><a name="method_graph_init_execution_context.0"></a> result&lt;own&lt;<a href="#graph_execution_context"><a href="#graph_execution_context"><code>graph-execution-context</code></a></a>&gt;, <a href="#error"><a href="#error"><code>error</code></a></a>&gt;</li>
</ul>
<h4><a name="load"></a><code>load: func</code></h4>
<p>Load a <a href="#graph"><code>graph</code></a> from an opaque sequence of bytes to use for inference.</p>
Expand All @@ -286,7 +266,7 @@ graph IR in parts (e.g., OpenVINO stores its IR and weights separately).</p>
</ul>
<h5>Return values</h5>
<ul>
<li><a name="load.0"></a> result&lt;own&lt;<a href="#graph"><a href="#graph"><code>graph</code></a></a>&gt;, own&lt;<a href="#error"><a href="#error"><code>error</code></a></a>&gt;&gt;</li>
<li><a name="load.0"></a> result&lt;own&lt;<a href="#graph"><a href="#graph"><code>graph</code></a></a>&gt;, <a href="#error"><a href="#error"><code>error</code></a></a>&gt;</li>
</ul>
<h4><a name="load_by_name"></a><code>load-by-name: func</code></h4>
<p>Load a <a href="#graph"><code>graph</code></a> by name.</p>
Expand All @@ -299,5 +279,5 @@ range from simple to complex (e.g., URLs?) and caching mechanisms of various kin
</ul>
<h5>Return values</h5>
<ul>
<li><a name="load_by_name.0"></a> result&lt;own&lt;<a href="#graph"><a href="#graph"><code>graph</code></a></a>&gt;, own&lt;<a href="#error"><a href="#error"><code>error</code></a></a>&gt;&gt;</li>
<li><a name="load_by_name.0"></a> result&lt;own&lt;<a href="#graph"><a href="#graph"><code>graph</code></a></a>&gt;, <a href="#error"><a href="#error"><code>error</code></a></a>&gt;</li>
</ul>
13 changes: 6 additions & 7 deletions wit/wasi-nn.wit
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,12 @@ interface errors {
unknown
}

resource error {
constructor(code: error-code, data: string);
record error {
/// The error code identifies the general reason the operation failed.
code: error-code,

/// Return the error code.
code: func() -> error-code;

/// Errors can propagated with backend specific status through a string value.
data: func() -> string;
/// The data field propagates the backend failure context as a string for easier
/// troubleshooting.
data: string,
}
}

0 comments on commit 22e4af8

Please sign in to comment.