Make compatible with thumbv6m-none-eabi

[BjornTheProgrammer]

Checklist

• Confirmed that run-checks all script has been executed.
• Made sure the book is up to date with changes in this PR.

Related Issues/PRs

Problem with no_std on thumbv6m-none-eabi #302
Inference on an embedded MCU (RP2040 / Raspberry Pico) #1067
[WIP] Use portable atomics to build for thumbv6m-none-eabi target #374

Changes

The goal is to get burn working on a thumbv6m-none-eabi target, which std lib does not have access to Arc, and some atomics.

It fixes issues surrounding compilation using a new version of ndarray and exposes some feature flags. It also provides an example model that runs on a thumbv6m-none-eabi target.

Testing

Haven't been able to test completely yet, due to cubecl and cubecl-common dependencies breaking the build.

[BjornTheProgrammer]

Checklist of things that need to be done

• pull request to cubecl and cubecl-common to fix errors with alloc

use alloc::{boxed::Box, sync::Arc, task::Wake, vec::Vec};
    |                         ^^^^       ^^^^^^^^^^ no `Wake` in `task`
    |                         |
    |                         could not find `sync` in `alloc`

• Fix testing, so that ./run-checks all uses the right target
• Remove all deprecated methods from ndarray being used
• Test on actual hardware

[antimora]

@BjornTheProgrammer FYI, it's sufficient to fix make this work with crates in this test: https://github.com/tracel-ai/burn/tree/main/crates/burn-no-std-tests

[BjornTheProgrammer]

I believe I have actually found the issue.

burn-core says this

[dependencies]

# ** Please make sure all dependencies support no_std when std is disabled **

burn-common = { path = "../burn-common", version = "0.14.0", default-features = false }
burn-dataset = { path = "../burn-dataset", version = "0.14.0", optional = true, default-features = false }
burn-derive = { path = "../burn-derive", version = "0.14.0" }
burn-tensor = { path = "../burn-tensor", version = "0.14.0", default-features = false }

# ...

But burn-common has a dependency cubecl-common = { workspace = true }, which is no_std, but doesn't work with no atomic ptr. Which then causes those errors

We could just make it so that cubcl-common is an optional dependency, or we could merge pull request #52 which changes cubcl-common when not std to use portable-atomic

[antimora]

Tagging @nathanielsimard @laggui @syl20bnr to suggest dependency consistency.

I will take a look closer tomorrow.

[BjornTheProgrammer]

This pull request only supports ndarray. Do you all think that exploration into other backend supports should be done, like autodiff or candle?

[antimora]

This pull request only supports ndarray. Do you all think that exploration into other backend supports should be done, like autodiff or candle?
We mainly target inference.

We do not have no-std tests for candle. It would be great if candle worked because it's CPU implentation is more optimized than ndarray.

Maybe you could tackle them in a different pr

[BjornTheProgrammer]

Tagging @nathanielsimard @laggui @syl20bnr to suggest dependency consistency.

I will take a look closer tomorrow.

Any resolution on this front?

[antimora]

Tagging @nathanielsimard @laggui @syl20bnr to suggest dependency consistency.
I will take a look closer tomorrow.

Any resolution on this front?

Yes, the cubecl-common has std flag enabled by default. You need to disable default features like this:

I resolved the conflicts of your PR locally and built and tested burn-no-std-tests successfully.

Here is the diff you need to apply:

Probably left out by recent refactoring.

[BjornTheProgrammer]

Based on my testing, this doesn't work. When you try to build for the thumbv6m-none-eabi target, you still get many atomic and arc errors.

To test yourself use the following command in burn-no-std-tests.
RUSTFLAGS="--cfg portable_atomic_unsafe_assume_single_core" cargo build --target thumbv6m-none-eabi

thumbv6m-none-eabi does not support atomic ptrs, despite thumbv7m-none-eabi having them. This is the fundamental issue here. Pull request #52 fixes that issue.

[BjornTheProgrammer]

For the following, I do not know which method is best used instead of the deprecated method. Your input would be appreciated.

[BjornTheProgrammer]

I've added documentation to the burn book (it should be rewritten more to your standards) and replaced the deprecated methods, I'm unsure if I did so correctly. One of the tests still fails from asserts with ndarray. However, it runs on the Raspberry Pi Pico just fine.

These are the final things that need to be fixed before this can be merged. @antimora would you mind tackling these?

[antimora]

I've added documentation to the burn book (it should be rewritten more to your standards) and replaced the deprecated methods, I'm unsure if I did so correctly. One of the tests still fails from asserts with ndarray. However, it runs on the Raspberry Pi Pico just fine.

These are the final things that need to be fixed before this can be merged. @antimora would you mind tackling these?

I will try. I have been feeling sick lately, so my availability might be sporadic.

[antimora]

@BjornTheProgrammer is this error you're gettin when building run-checks.sh no-std?

error: dependents require atomic CAS but not available on this target by default;
       consider enabling one of the `unsafe-assume-single-core` or `critical-section` Cargo features.
       see <https://docs.rs/portable-atomic/latest/portable_atomic/#optional-features> for more.
   --> /Users/dilshod/.cargo/registry/src/index.crates.io-6f17d22bba15001f/portable-atomic-1.7.0/src/lib.rs:424:1
    |
424 | / compile_error!(
425 | |     "dependents require atomic CAS but not available on this target by default;\n\
426 | |     consider enabling one of the `unsafe-assume-single-core` or `critical-section` Cargo features.\n\
427 | |     see <https://docs.rs/portable-atomic/latest/portable_atomic/#optional-features> for more."
428 | | );
    | |_^

error: could not compile `portable-atomic` (lib) due to 1 previous error
warning: build failed, waiting for other jobs to finish...
[burn]%

[antimora]

I resolved conflicts and rebase. Let me know what's blocking you.

[BjornTheProgrammer]

I fixed that error, just by updating the revision of cubecl. This is the error I'm getting that I don't know how to fix.

Upon fixing this, the PR should be ready to merge. I'll open this to no longer be a draft.

---- ops::base::tests::should_generate_row_major_layout_for_cat stdout ----
thread 'ops::base::tests::should_generate_row_major_layout_for_cat' panicked at crates/burn-ndarray/src/ops/base.rs:653:9:
assertion failed: array.array.is_standard_layout()

[BjornTheProgrammer]

@antimora Fixed all issues, ./run-checks.sh all works as expected. I have tested on actual hardware. This pull request should be ready to merge.

[antimora]

@BjornTheProgrammer i will review in coming days. I also invited others to look into this. This will be a very useful feature especially with your example and a book section!

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.04%. Comparing base (e1fed79) to head (dd0489e).
Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2096   +/-   ##
=======================================
  Coverage   86.04%   86.04%           
=======================================
  Files         695      695           
  Lines       88882    88882           
=======================================
  Hits        76480    76480           
  Misses      12402    12402           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[laggui]

This looks awesome! Thanks for the great addition including both an example and step-by-step guide in the book 🙂

I'll make time to review this in the next few days 🙏

[antimora]

LGTM

Thank you for you addition. This feature has been asked a few times. I am glad we support CPUs without atomic instructions.

[BjornTheProgrammer]

@antimora @laggui I've rebased the commit to the latest on the main, I've also renamed and reordered the commits. Nothing has changed in the content.

[antimora]

@laggui I hope you'd be able to review it soon so we can merge it before other deviations appear on the main.

[laggui]

Wow I reviewed this earlier this afternoon, I guess I forgot to submit 😅

[laggui]

Awesome work!

Everything looks good, I just have a minor request:

Can you add a small README to the example, and perhaps rename it to something more explicit instead of rp2040. know it is for the target embedded system, but perhaps something like raspberry-pi-micro would be better?

I'll let you decide what you think fits best 😄

[BjornTheProgrammer]

@laggui I've made the requested changes! Good catch, I didn't think of that at all!

[laggui]

Thanks for the quick change! Looks good to go 👌🏻

[BjornTheProgrammer]

Thank you so much everyone. I really appreciate your project! It's bringing forward the possibilities of ai inference!