-
Notifications
You must be signed in to change notification settings - Fork 57
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
Add in and out degree for states #45
Draft
jeffa5
wants to merge
10
commits into
stateright:master
Choose a base branch
from
jeffa5:degree
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
af28419
Add degree calculations
272cfb9
Add basic in-degree
443617b
Fix tests
d9c0337
Track in_degree and out_degree in more detail
6eab61b
Update test
48210eb
Update comments
3d45939
Pre-filter next_states
c45e517
Fix average calculations and count out degree after filtering next
f1ee2ce
Fix average and track all actions in out degree
6dd8791
Fix test
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,13 @@ use std::sync::atomic::{AtomicUsize, Ordering}; | |
// While this file is currently quite similar to dfs.rs, a refactoring to lift shared | ||
// behavior is being postponed until DPOR is implemented. | ||
|
||
#[derive(Default)] | ||
struct NodeData { | ||
in_degree: usize, | ||
out_degree: usize, | ||
parent_fingerprint: Option<Fingerprint>, | ||
} | ||
|
||
pub(crate) struct BfsChecker<M: Model> { | ||
// Immutable state. | ||
model: Arc<M>, | ||
|
@@ -25,7 +32,7 @@ pub(crate) struct BfsChecker<M: Model> { | |
job_market: Arc<Mutex<JobMarket<M::State>>>, | ||
state_count: Arc<AtomicUsize>, | ||
max_depth: Arc<AtomicUsize>, | ||
generated: Arc<DashMap<Fingerprint, Option<Fingerprint>, BuildHasherDefault<NoHashHasher<u64>>>>, | ||
generated: Arc<DashMap<Fingerprint, NodeData, BuildHasherDefault<NoHashHasher<u64>>>>, | ||
discoveries: Arc<DashMap<&'static str, Fingerprint>>, | ||
} | ||
struct JobMarket<State> { wait_count: usize, jobs: Vec<Job<State>> } | ||
|
@@ -50,7 +57,7 @@ where M: Model + Send + Sync + 'static, | |
let max_depth = Arc::new(AtomicUsize::new(0)); | ||
let generated = Arc::new({ | ||
let generated = DashMap::default(); | ||
for s in &init_states { generated.insert(fingerprint(s), None); } | ||
for s in &init_states { generated.insert(fingerprint(s), NodeData::default()); } | ||
generated | ||
}); | ||
let ebits = { | ||
|
@@ -176,7 +183,7 @@ where M: Model + Send + Sync + 'static, | |
fn check_block( | ||
model: &M, | ||
state_count: &AtomicUsize, | ||
generated: &DashMap<Fingerprint, Option<Fingerprint>, BuildHasherDefault<NoHashHasher<u64>>>, | ||
generated: &DashMap<Fingerprint, NodeData, BuildHasherDefault<NoHashHasher<u64>>>, | ||
pending: &mut Job<M::State>, | ||
discoveries: &DashMap<&'static str, Fingerprint>, | ||
visitor: &Option<Box<dyn CheckerVisitor<M> + Send + Sync>>, | ||
|
@@ -255,6 +262,11 @@ where M: Model + Send + Sync + 'static, | |
// Otherwise enqueue newly generated states (with related metadata). | ||
let mut is_terminal = true; | ||
model.actions(&state, &mut actions); | ||
let generated_fingerprint = fingerprint(&state); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you reuse |
||
generated | ||
.entry(generated_fingerprint) | ||
.and_modify(|nd| nd.out_degree = actions.len()); | ||
|
||
let next_states = actions.drain(..).flat_map(|a| model.next_state(&state, a)); | ||
for next_state in next_states { | ||
// Skip if outside boundary. | ||
|
@@ -270,19 +282,29 @@ where M: Model + Send + Sync + 'static, | |
// that it holds in the path leading to the second visit -- another | ||
// possible false-negative. | ||
let next_fingerprint = fingerprint(&next_state); | ||
if let Entry::Vacant(next_entry) = generated.entry(next_fingerprint) { | ||
next_entry.insert(Some(state_fp)); | ||
} else { | ||
// FIXME: arriving at an already-known state may be a loop (in which case it | ||
// could, in a fancier implementation, be considered a terminal state for | ||
// purposes of eventually-property checking) but it might also be a join in | ||
// a DAG, which makes it non-terminal. These cases can be disambiguated (at | ||
// some cost), but for now we just _don't_ treat them as terminal, and tell | ||
// users they need to explicitly ensure model path-acyclicality when they're | ||
// using eventually properties (using a boundary or empty actions or | ||
// whatever). | ||
is_terminal = false; | ||
continue | ||
match generated.entry(next_fingerprint) { | ||
Entry::Occupied(mut o) => { | ||
let nd = o.get_mut(); | ||
nd.in_degree += 1; | ||
// FIXME: arriving at an already-known state may be a loop (in which case it | ||
// could, in a fancier implementation, be considered a terminal state for | ||
// purposes of eventually-property checking) but it might also be a join in | ||
// a DAG, which makes it non-terminal. These cases can be disambiguated (at | ||
// some cost), but for now we just _don't_ treat them as terminal, and tell | ||
// users they need to explicitly ensure model path-acyclicality when they're | ||
// using eventually properties (using a boundary or empty actions or | ||
// whatever). | ||
is_terminal = false; | ||
continue | ||
}, | ||
Entry::Vacant(v) => { | ||
let nd = NodeData { | ||
in_degree: 1, | ||
out_degree: 0, | ||
parent_fingerprint: Some(state_fp), | ||
}; | ||
v.insert(nd); | ||
}, | ||
} | ||
|
||
// Otherwise further checking is applicable. | ||
|
@@ -322,6 +344,20 @@ where M: Model, | |
self.max_depth.load(Ordering::Relaxed) | ||
} | ||
|
||
fn out_degrees(&self) -> Vec<usize>{ | ||
self.generated | ||
.iter() | ||
.map(|kv| kv.value().out_degree) | ||
.collect() | ||
} | ||
|
||
fn in_degrees(&self) -> Vec<usize>{ | ||
self.generated | ||
.iter() | ||
.map(|kv| kv.value().in_degree) | ||
.collect() | ||
} | ||
|
||
fn discoveries(&self) -> HashMap<&'static str, Path<M::State, M::Action>> { | ||
self.discoveries.iter() | ||
.map(|mapref| { | ||
|
@@ -349,7 +385,7 @@ where M: Model, | |
|
||
fn reconstruct_path<M>( | ||
model: &M, | ||
generated: &DashMap<Fingerprint, Option<Fingerprint>, BuildHasherDefault<NoHashHasher<u64>>>, | ||
generated: &DashMap<Fingerprint, NodeData, BuildHasherDefault<NoHashHasher<u64>>>, | ||
fp: Fingerprint) | ||
-> Path<M::State, M::Action> | ||
where M: Model, | ||
|
@@ -363,7 +399,7 @@ fn reconstruct_path<M>( | |
let mut fingerprints = VecDeque::new(); | ||
let mut next_fp = fp; | ||
while let Some(source) = generated.get(&next_fp) { | ||
match *source { | ||
match source.parent_fingerprint { | ||
Some(prev_fingerprint) => { | ||
fingerprints.push_front(next_fp); | ||
next_fp = prev_fingerprint; | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing impacting performance is the change from storing
Option<Fingerprint>
in the map (which encodes to a singleu64
asFingerprint
is aNonZeroU64
enabling a memory layout optimization) to this new structure that cannot encode as efficiently.I don't know whether it would help or hurt performance, but perhaps adopting different maps along the lines of https://en.wikipedia.org/wiki/Data-oriented_design is worth exploring.