Skip to content
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

Fix no default features flags + update cubecl #2725

Merged
merged 4 commits into from
Jan 21, 2025

Conversation

laggui
Copy link
Member

@laggui laggui commented Jan 21, 2025

Checklist

  • Confirmed that run-checks all script has been executed.

Related Issues/PRs

#2709
CubeCL #438
HIP sys #13

Changes

When specifying no default features some crates would inevitably enable the downstream default features for cubecl, which causes issues when trying to specify a conflicting feature. Although feature flags should be additive, this is not exactly the case for cubecl-hip (multiple version flags will fail to build), so for now we can make sure that the downstream flags correspond.

When specifying no default features some crates would inevitably enable the downstream default features for cubecl, which causes issues when trying to specify a conflicting feature (e.g., std default)
Copy link

codecov bot commented Jan 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.59%. Comparing base (140ea75) to head (15be0c6).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2725      +/-   ##
==========================================
- Coverage   83.60%   83.59%   -0.02%     
==========================================
  Files         819      819              
  Lines      106600   106600              
==========================================
- Hits        89124    89113      -11     
- Misses      17476    17487      +11     

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

@laggui
Copy link
Member Author

laggui commented Jan 21, 2025

Seems that we hit the default recursion limit (128) with the new wgpu 0.24 types. Haven't spent much time to dig into it but simply bumping the limit works 🫤

diff --git a/crates/burn-core/src/lib.rs b/crates/burn-core/src/lib.rs
index f5545184..ade8d64d 100644
--- a/crates/burn-core/src/lib.rs
+++ b/crates/burn-core/src/lib.rs
@@ -1,6 +1,7 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 #![warn(missing_docs)]
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![recursion_limit = "135"]

 //! The core crate of Burn.

diff --git a/crates/burn-router/src/lib.rs b/crates/burn-router/src/lib.rs
index 644f65ee..773235f7 100644
--- a/crates/burn-router/src/lib.rs
+++ b/crates/burn-router/src/lib.rs
@@ -1,6 +1,7 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 #![warn(missing_docs)]
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![recursion_limit = "138"]

 //! Burn multi-backend router.

diff --git a/examples/guide/src/bin/infer.rs b/examples/guide/src/bin/infer.rs
index 3c64879b..6a246d85 100644
--- a/examples/guide/src/bin/infer.rs
+++ b/examples/guide/src/bin/infer.rs
@@ -1,3 +1,4 @@
+#![recursion_limit = "131"]
 use burn::{backend::Wgpu, data::dataset::Dataset};
 use guide::inference;

diff --git a/examples/image-classification-web/src/lib.rs b/examples/image-classification-web/src/lib.rs
index 3881123e..3d528f2e 100644
--- a/examples/image-classification-web/src/lib.rs
+++ b/examples/image-classification-web/src/lib.rs
@@ -1,4 +1,5 @@
 #![cfg_attr(not(test), no_std)]
+#![recursion_limit = "135"]

 pub mod model;
 pub mod web;
diff --git a/examples/server/src/lib.rs b/examples/server/src/lib.rs
index 92cba57a..70705a08 100644
--- a/examples/server/src/lib.rs
+++ b/examples/server/src/lib.rs
@@ -1,3 +1,5 @@
+#![recursion_limit = "141"]
+

@nathanielsimard nathanielsimard merged commit b33bd24 into main Jan 21, 2025
11 checks passed
@nathanielsimard nathanielsimard deleted the fix/no-default-features branch January 21, 2025 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants