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

Automatically enable autotune_persistent_cache only on desktop platforms #69

Merged
merged 4 commits into from
Aug 13, 2024

Conversation

ArthurBrussee
Copy link
Contributor

@ArthurBrussee ArthurBrussee commented Aug 13, 2024

Currently, the persistent cache fails on platforms without easy access to a filesystem like android and the web. Configuring this as a negative feature becomes quite annoying.

Instead, as per this discord discussion, this just makes it so it's only enabled if on a desktop platform. To save some gnarly cfgs everywhere this using cfg_aliases which might be useful more generally in cube/burn, but lmk if it's preferred to not have this.

There's also a leftover flag in cubecl-cuda for autotune I think. It seems Burn actually handles all the autotune flags, cube doesn't know about them.

@ArthurBrussee ArthurBrussee changed the title Make autotune_persistent_cache a config that's automatically only desktop enabled Automatically enable autotune_persistent_cache only on desktop platforms Aug 13, 2024
@@ -80,16 +80,22 @@ impl VariableAnalyzer {
for pat in pat_tuple.elems.iter() {
let (id, is_comptime) = find_local_declaration_ident(pat);
if let Some(id) = id {
self.variable_tracker
.analyze_declare(id.to_string(), depth, is_comptime);
self.variable_tracker.analyze_declare(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just to keep CI happy, seems some bad formatting was comitted.

Copy link
Member

@nathanielsimard nathanielsimard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines +5 to +7
cfg_aliases! {
autotune_persistent_cache: { all(feature = "std", any(target_os = "windows", target_os = "linux", target_os = "macos")) },
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ho I didn't know we could do that, awesome!

@nathanielsimard nathanielsimard merged commit 034f667 into tracel-ai:main Aug 13, 2024
1 of 2 checks passed
@ArthurBrussee ArthurBrussee deleted the autotune-cache-fix branch October 16, 2024 19:55
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