huggingface / huggingface/candle

Proposal to add an optional `iex` dpendency

Open
#3,276 4 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
21k
Forks
1.8k
Avg merge
16h 42m
Merged PRs (30d)
25

Description

Most of the functions in `candle` are fallible, even though in most cases a working program runs through a happy path only. The `Result<_, _>` type has a tendency to add some measurable overhead in cases like this.

There is a crate, called [`iex`](https://docs.rs/iex/latest/iex/index.html), that addresses this point, by converting `Err` returns into panics and `Ok` returns into (essentially) bare values, via `#[iex]` attribute macro. It is still possible to obtain the `Result` via `.into_result()` method on the returned value (through the `catch_unwind` mechanism).

I'd like to try introducing the `iex` crate into the `candle` as an optional dependency, that would ultimately require in most cases to just annotate the fallible function with `#[cfg_attr(feature = "iex", iex)]`, and maybe perform some minor rewrites (e.g. doing `return Ok(fallible?)` instead of `return fallible`, or manually collecting a `Result>` in a loop).

At the moment, I'm trying to reach proof-of-concept state.

I got "stuck" on the implementations of operator traits on `Tensor`, like `Add`, `Mul` etc.
Unfortunately, the resulting return type of `#[iex]`-annotated functions is an opaque `impl Outcome<_>`, and it is not yet possible to use `impl Trait` in associated types, so I'm unable to set the `Output` type's value to it.
I say "stuck", because I could just use `.into_result()`, but it's not recommended to be used in hot paths, so idk.

In any way, benchmarking should be done.

I'll open a draft PR if you are interested.

Also, I've just discovered that `CustomOp`s have to be `dyn`-compatible, which won't be the case if methods become annotated with `#[iex]`.

Potentially the `iex` crate might need to be forked to support the `use<>` notation (and `candle` updated to MSRV 1.82)

Contributor guide

No contributing guide indexed for this repository

Research direction

Review the Tensor operator trait implementations and CustomOp methods mentioned in the issue, then benchmark the current Result-based paths against an optional iex path. Done means a viable integration that handles operator output types and dyn-compatible CustomOps, with benchmark results.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
machine-learning, performance
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.