Conversation
|
Also, is rust 1.15 support needed? The nested imports only work from 1.25 onwards. https://rust-lang-nursery.github.io/edition-guide/rust-2018/module-system/nested-imports-with-use.html |
|
I just realized that fixedbitset uses std also so I made a pull request for adding no_std support to it here: petgraph/fixedbitset#31 |
|
Hi, what's the goal and motivation of this change? What's the new thing we can do with this, and are there alternative ways of achieving it? |
bluss
left a comment
There was a problem hiding this comment.
It would be good to have the motivations & goals part filled out. I think petgraph can not accept losing HashMap, so this approach does not seem viable to me. If we want to have certain functionality available in no_std environments, there may be other approaches.
|
The original reason for doing this was because of a commercial project that used petgraph for handling graphs that were central to the the application. One of the requirements was that the binary had to run on embedded devices as well as on normal operating systems. Hence the need for no_std support. I too think that it would be best if HashMap was available in alloc::collections. Maybe this PR should wait for when/if this one is merged? rust-lang/rust#56192 |
|
It does make me realize how useful indexmap on no std would be, too |
|
After pondering this for a while, maybe it would be best if I changed the implementation to use the hashbrown crate https://github.com/rust-lang/hashbrown ? It supports no_std and it should be virtually the same std lib hashmap. |
|
I had a look at switching out the underlying map from Can be re-implemented, but there are performance considerations:
Cannot be implemented without
We could do some cfg attribute magic to use hashbrown with |
|
One could jettison graphmap if we don't have std. That datastructure needs a lot of love (we have a rewrite issue #208). Note, we have a fruitful WIP on making indexmap no-std. |
|
Also not having noticed that before now, the usual solution is a default feature named "std". cargo features must be additive, we can't have a cfg flag that takes something away. |
|
@jonimake I've rebased this PR over master. There were quite a few changes, so you might want to reset your branch on my branch's head. Since it's been a while since you opened this PR, I can close this one and take over if you like. |
|
Right I reset my branch to that and it seems to compile fine on my end, travis seems to be quite broken though. No idea about that. |
|
Yeah I just had a look & it seems to be the diff --git a/src/simple_paths.rs b/src/simple_paths.rs
index 7f49b3b..6843588 100644
--- a/src/simple_paths.rs
+++ b/src/simple_paths.rs
@@ -98,7 +98,7 @@ mod test {
use core::iter::FromIterator;
#[cfg(not(feature = "alloc"))]
- use std::{collections::HashSet, iter::FromIterator};
+ use std::{collections::HashSet};
#[cfg(feature = "alloc")]
use alloc::{collections::BTreeSet as HashSet, vec::Vec};
@@ -107,6 +107,9 @@ mod test {
use crate::prelude::DiGraph;
+ #[cfg(not(feature = "no_std"))]
+ use crate::dot::Dot;
+
use super::all_simple_paths;
#[test] |
|
While the changes here could still use some refactoring, I think we only need to flip |
|
So to sum it up:
Did I understand that right? What about the |
|
Looks like the failing travis test fails due to the all part in Edit: Looks like it's the same as |
|
Could you please rebase this one? I think it will be very cool addition to the future 0.6 release. |
This adds a new crate for working with dynamic audio graphs. From the new docs: > `dasp_graph` is targeted towards users who require an efficient yet > flexible and dynamically configurable audio graph. Use cases might > include virtual mixers, digital audio workstations, game audio systems, > virtual modular synthesizers and related usecases. This work has been a long-time coming and is the result of many discussions in the rust audio community and many lessons learned over the last few years of working with rust and audio. In particular: - the development of and reflection on [dsp-chain](https://crates.io/crates/dsp-chain) and its shortcomings. - The (reasonable) limitations of [dasp_signal](https://crates.io/crates/dasp_signal) when dynamically configuring graphs. - Discussion on the design of audio graphs with @raphlinus at RustAudio/dsp-chain#141. - The development of the [spatial audio server](https://github.com/museumsvictoria/spatial_audio_server). - A recent email discussion with Sami Perttu on DASP and audio graphs. `dasp_graph` is of course not a one-size-fits-all solution. Instead, it is designed specifically to work well alongside (and fill a gap within) the rest of the `dasp` crate ecosystem. Please refer to the "Comparing `dasp_signal`" section of the `dasp_graph` root documentation for a more detailed overview of the design choices between the two, what applications each are best suited toward and how the two best interoperate together. A small suite of node implementations are provided out of the box including a `Delay`, `Sum`, `Pass`, `GraphNode` and `BoxedNode`, all of which can be enabled/disabled via their associated features. Following this, I have some ideas for adding an optional `sync` module to the crate, aimed at controlling and monitoring a dasp graph and it's nodes from a separate thread (i.e. for convenient use alongside a GUI) in a non-dynamically-allocating, non-blocking manner. The work so far has been performed with these plans in mind. The ideas are for the most part based on the discussion at RustAudio/dsp-chain#141. Also, `no_std` support for `dasp_graph` is currently blocked on petgraph support for `no_std`. A PR is open for adding `no_std` support at petgraph/petgraph#238. In the meantime, the `std` feature must be enabled to use the new `dasp::graph` module. This is also noted in the updated docs. For more information about the crate and inner workings feel free to read through the new `dasp_graph` docs. I'm yet to add examples, but hopefully the added tests can give a good idea of how to use the crate in the meantime.
|
I was planning on implementing no_std myself, but I have found this. |
|
I have merged current master to no_std |
Using this feature switch will also turn on alloc feature switch which is used to determine if the collections used in the library should be imported from the alloc library or not. Most notable change in no_std is that instead of a HashMap and Set a BTreeMap and Set are used. Other squashed changes: - Added cfg_attr for switching on nightly alloc feature - Added nightly travis test with no_std feature enabled - Fixed travis.yml syntax - Use fixedbitset with no_std feature in no_std mode - Run cargo fmt
This comment has been minimized.
This comment has been minimized.
|
I have started to rewrite it from the beggining for 'no_std'.
|
|
@XVilka I haven't thought that it might be in other mod than hash_map . Thanks |
|
I have updated it, and now it passes no_std check (https://github.com/mystor/cargo-no-std-check) zendurix@ef0eded |
|
@zendurix please do |
|
Hi, is there any update on this? Thanks! |
|
0.7.0 will have no-std support, until this version has landed I will keep this PR open. |
# Objective - Alternative to #238 - Alternative to #370 - Implements Item 6 from #551 ## Solution - Promoted `hashbrown` to a direct dependency (currently transient through `indexmap`) - Added a new `std` feature, and included it in all features which rely on `std` for documentation purposes. - Added new CI task to ensure `no_std` support works (using `wasm32v1-none` as an example `no_std` target) - Added several lints which help minimise `std` usage - Adjusted certain public APIs to allow passing a `S: BuildHasher` instead of implicitly relying on `std::hash::RandomState` when the `std` feature is disabled --- ## Notes I know this has been attempted several times before and delayed due to a desire to refactor the crate _first_ before adding this functionality. Delays in adding `no_std` support forced Bevy to drop `petgraph` from its core crates, relying on a specialised alternative. We'd love to keep using `petgraph`, but `no_std` support is now a requirement going forward. This will also come up with `wgpu` which is _also_ pursuing `no_std` support [here](gfx-rs/wgpu#6826). --- BREAKING CHANGE: Petgraph previously assumed the usage of `std::hash::RandomState` as the hashing implementation across its codebase. In `no_std`, this is not possible. To minimise friction for libraries supporting both `std` and `no_std` with Petgraph, I have made the following changes to the public API: ### `petgraph::algo::simple_paths::all_simple_paths` This function now has a 3rd generic parameter `S` which controls the hashing implementation used. Because `all_simple_paths` is a function it cannot have default generic parameters, so users must specify the hasher themselves. ```rust // Before let foo = all_simple_paths(/* ... */); // After let foo = all_simple_paths::<_, _, std::hash::RandomState>(/* ... */); ``` ### Switched from `std::collections::{HashMap, HashSet}` to `hashbrown::{HashMap, HashSet}` To support `no_std`, we cannot use the standard library's implementations of `HashMap` or `HashSet`. Methods and types previously referencing those collections from `std` will now reference them from `hashbrown`. Note that `hashbrown` was already a dependency of Petgraph, so no change in audit requirements. ### Added Hashing Parameter The following types have had a new generic parameter `S` added to specify the hashing implementation. Note that when `std` is enabled, these will all default to `std::hash::RandomState`, as before. - `UnGraphMap` - `DiGraphMap` - `MatrixGraph` - `DiMatrix` - `UnMatrix` - `NodeIdentifiers` - `NodeReferences` Note that for `MatrixGraph`, `DiMatrix`, `UnMatrix` the new `S` parameter is in the 3rd position (all others have `S` in the last position). The reason is `S` has a default parameter with `std` enabled, but is required with `std` disabled. This means it must be the last _required_ parameter. ```rust // Before let foo: MatrixGraph<Foo, Bar, Directed, Option<Bar>, DefaultIx>; // After let foo: MatrixGraph<Foo, Bar, std::hash::RandomState, Directed, Option<Bar>, DefaultIx>; ``` Also note that because `Default` can now be implemented for multiple versions of the above types (generic over the hasher), you may need to either specify the hasher, or explicitly declare it as default (with the `std` feature enabled): ```rust // Note that N and E are inferred by code below. // Before let foo = UnMatrix::default(); // After (Explicitly infer N and E, but use defaults otherwise) let foo = UnMatrix::<_, _>::default(); ``` ### `default-features = false` MSRV is 1.81 If you don't enable the `std` feature, the MSRV increases to 1.81, when `core::error::Error` was stabilised. To preserve the original MSRV of 1.64, just enable the `std` feature.
|
The PR #747 seems to have introduced the same behaviour. Thus I'll nominate this as to be closed :) |
|
Support for |
Using this feature switch will also turn on alloc feature switch which
is used to determine if the collections used in the library should be
imported from the alloc library or not.
Most notable change in no_std is that instead of a HashMap and Set
a BTreeMap and Set are used.
Benchmarks
Standard
No_std