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.

Sign up for GitHub

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

seq WASM compatibility #33

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

seq WASM compatibility #33

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6a215ad
seq WASM compatibility
celaus Jul 3, 2018
1434f0b
Finished an interface for collecting statistics
celaus Aug 7, 2018
cac99d4
code deduplication
celaus Aug 7, 2018
7663128
adjusted the statscollector trait
celaus Aug 7, 2018
6c7329c
fixed conditional build
celaus Aug 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ license = "MIT/Apache-2.0"
documentation = "https://docs.rs/rsgenetic/"

[dependencies]
rand = "0.3"
rand = "^0.5"
rayon = "1.0.0"
23 changes: 6 additions & 17 deletions src/sim/seq.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@ use super::*;
use super::select::*;
use super::iterlimit::*;
use super::earlystopper::*;
use std::time::Instant;
use std::boxed::Box;
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure why you need this but it might be needed somewhere. Just make sure to remove unused use statements if this isn't needed 😄

Copy link
Author

Choose a reason for hiding this comment

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

True laziness on my side. will be gone soon 👍

use std::marker::PhantomData;
use rand::prng::XorShiftRng;


/// A sequential implementation of `::sim::Simulation`.
/// The genetic algorithm is run in a single thread.
Expand All @@ -42,7 +44,6 @@ where
iter_limit: IterLimit,
selector: Box<Selector<T, F>>,
earlystopper: Option<EarlyStopper<F>>,
duration: Option<NanoSecond>,
error: Option<String>,
phantom: PhantomData<&'a T>,
}
Expand All @@ -63,15 +64,13 @@ where
iter_limit: IterLimit::new(100),
selector: Box::new(MaximizeSelector::new(3)),
earlystopper: None,
duration: Some(0),
error: None,
phantom: PhantomData::default(),
},
}
}

fn step(&mut self) -> StepResult {
let time_start;

if self.population.is_empty() {
self.error = Some(
Expand All @@ -88,8 +87,7 @@ where
};

if !should_stop {
time_start = Instant::now();


let mut children: Vec<T>;
{
// Perform selection
Expand Down Expand Up @@ -120,15 +118,6 @@ where
}

self.iter_limit.inc();
self.duration = match self.duration {
Copy link
Owner

Choose a reason for hiding this comment

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

Rather than remove this code, I would refactor it to a free function (not exported) and have that function have a different implementation based on the target. The regular implementation would be this removed code, while the WASM implementation would simply return None.

Could you also tell me how you compiled this project for WASM (I tried building it and got over 90 errors for undefined identifiers, such as c_long)?

Copy link
Author

Choose a reason for hiding this comment

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

I'll get something up soon that demonstrates that

Copy link
Author

Choose a reason for hiding this comment

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

about the Instant stuff, it could be as easy as an if/else Option each time, I just thought that there might be a better idea 😆 I guess we can improve later on as well though

Copy link
Owner

@m-decoster m-decoster Jul 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I'll get something up soon that demonstrates that

Thanks 👍. That will allow me to help you better.

about the Instant stuff [...]

I don't really understand what you mean here, but we can take a look at it once I get your PR to compile

Copy link
Author

Choose a reason for hiding this comment

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

I successfully build it like that:

cargo +nightly build --target wasm32-unknown-unknown
wasm-bindgen target/wasm32-unknown-unknown/release/wasm_tspsolver.wasm  --nodejs

Some(x) => {
let elapsed = time_start.elapsed();
let y = elapsed.as_secs() as NanoSecond * 1_000_000_000
+ u64::from(elapsed.subsec_nanos()) as NanoSecond;
Some(x + y)
}
None => None,
};

StepResult::Success // Not done yet, but successful
} else {
Expand Down Expand Up @@ -169,7 +158,7 @@ where
}

fn time(&self) -> Option<NanoSecond> {
self.duration
None
}

fn population(&self) -> Vec<T> {
Expand All @@ -185,7 +174,7 @@ where
/// Kill off phenotypes using stochastic universal sampling.
fn kill_off(&mut self, count: usize) {
let ratio = self.population.len() / count;
let mut i = ::rand::thread_rng().gen_range::<usize>(0, self.population.len());
let mut i = XorShiftRng::new_unseeded().gen_range::<usize>(0, self.population.len());
Copy link
Owner

Choose a reason for hiding this comment

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

It looks like new_unseeded is deprecated and you should use the FromEntropy or SeedableRng trait instead.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I know - fromEntropy won't work with WASM though, I was trying to find something that is sufficiently random and available, but I think it will come back to the user to take care of seeding it properly. This is an architecture issue, I'd say - because PRNGs will repeat after a while which means passing in a RNG is going to different than what it is now (I think).

But in any case, passing in a seed will need an interface, the question is where 😄

Copy link
Owner

Choose a reason for hiding this comment

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

I would think that passing in the seed would go well in the SimBuilder. You could have a method that sets the seed, and have a default seed as well. That way you should be able to use Seedable, not break the interface, and remove this deprecated call 😃

for _ in 0..count {
self.population.swap_remove(i);
i += ratio;
Expand Down