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

Forward std feature to some deps. #1321

Merged
merged 2 commits into from
Dec 11, 2023
Merged

Forward std feature to some deps. #1321

merged 2 commits into from
Dec 11, 2023

Conversation

zachs18
Copy link
Contributor

@zachs18 zachs18 commented Nov 17, 2023

Fixes #623 (specifically about Complex::from_polar at least)

nalgebra's std cargo feature now also enables the std feature of some dependencies that use default-features = false (num-traits, num-rational, num-complex, approx, (if enabled) alga)

(Note: the "alga?/std" syntax for enabling features of optional dependencies without necessarily enabling the optional dependency itself was only stabilized in Rust 1.60, but nalgebra seems to state that it's MSRV policy is "latest stable" so that would be fine.)

`nalgebra`'s `std` cargo feature now also enables the `std` feature of some dependencies that use `default-features = false`
Copy link
Collaborator

@Ralith Ralith left a comment

Choose a reason for hiding this comment

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

Thanks!

@Ralith
Copy link
Collaborator

Ralith commented Dec 10, 2023

Looks like the CUDA test in CI needs to be updated to support the optional dependency feature syntax. Want to have a look at that?

@zachs18
Copy link
Contributor Author

zachs18 commented Dec 11, 2023

It looks like rust-CUDA/cuda_std (which the nalgebra/cuda feature enables via simba/cuda) currently only supports nightly-2021-12-04 (Rust-GPU/Rust-CUDA#98), which is too old to support the ?/ syntax. Recent nightlys fail since cuda_std uses nightly features which have been removed. (https://github.com/zachs18/nalgebra/actions/runs/7161897944/job/19498133305 running on nightly-2023-12-10)

I don't see a way to resolve this with both rust-CUDA support (in CI or otherwise) and alga?/std in Cargo.toml.

I guess alga?/std can be removed, and alga will just not have the std feature enabled by nalgebra's std feature? The others (num-traits, num-complex, num-rational, approx, and simba) will still have their std features enabled. Does that sound fine?

nightly-2021-12-04 (used for cuda) is too old to support it.
@zachs18
Copy link
Contributor Author

zachs18 commented Dec 11, 2023

Note that the cuda CI job will fail anyway as it has before due to version resolution https://github.com/dimforge/nalgebra/actions/runs/6839550246/job/18605831607 (probably related to rust-lang/cargo#10623?)

@Ralith
Copy link
Collaborator

Ralith commented Dec 11, 2023

Ah, yeah, looks like Rust-CUDA is de facto unmaintained :(

I saw some mention of https://github.com/coreylowman/cudarc, maybe we should port to that? But that's out of scope here.

I guess alga?/std can be removed, and alga will just not have the std feature enabled by nalgebra's std feature? The others (num-traits, num-complex, num-rational, approx, and simba) will still have their std features enabled. Does that sound fine?

Sounds good to me. I'd prefer to drop CUDA support and hit all the deps, but that's @sebcrozet's call, and in the mean time this is a strict improvement. Alga users can enable std on that dep themselves if they need it.

@Ralith Ralith merged commit a01fa48 into dimforge:dev Dec 11, 2023
11 of 12 checks passed
@sebcrozet
Copy link
Member

@Ralith Updating CUDA to the latest rustc/llvm version is a very difficult piece of work so it will probably not happen any time soon (if ever). So let’s just remove CUDA support. Users of cuda can pin the dependency of nalgebra that supported it.

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.

Some std features of external crates aren't re-exported
3 participants