Skip to content

Commit

Permalink
Add check for state keys duplication
Browse files Browse the repository at this point in the history
  • Loading branch information
reinterpretcat committed Jun 4, 2024
1 parent 84a9c6c commit 5b39ff3
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 1 deletion.
19 changes: 18 additions & 1 deletion vrp-core/src/models/goal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ pub struct GoalContextBuilder {
impl GoalContextBuilder {
/// Creates a `GoalBuilder` with the given list of features.
pub fn with_features(features: Vec<Feature>) -> GenericResult<Self> {
let ids_all = features.iter().map(|feature| feature.name.clone()).collect::<Vec<_>>();
let ids_all = features.iter().map(|feature| feature.name.as_str()).collect::<Vec<_>>();
let ids_unique = ids_all.iter().collect::<HashSet<_>>();

if ids_unique.len() != ids_all.len() {
Expand All @@ -89,6 +89,23 @@ impl GoalContextBuilder {
)
.into());
}
drop(ids_unique);

// check state keys duplication
let state_keys = features
.iter()
.filter_map(|f| f.state.as_ref().map(|state| (f.name.clone(), state.state_keys().collect::<HashSet<_>>())))
.collect::<Vec<_>>();
state_keys.iter().try_for_each(|(outer_name, outer_keys)| {
state_keys.iter().filter(|(inner_name, _)| inner_name != outer_name).try_for_each(
|(inner_name, inner_keys)| {
inner_keys.intersection(outer_keys).next().map_or(Ok(()), |_| {
Err(format!("feature {outer_name} and {inner_name} shares common state keys for caching"))
})
},
)
})?;
drop(state_keys);

Ok(Self { main_goal: None, alternative_goals: Vec::default(), features })
}
Expand Down
40 changes: 40 additions & 0 deletions vrp-core/tests/unit/models/goal_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,3 +199,43 @@ fn can_use_objective_total_order_impl(left_fitness: Vec<f64>, right_fitness: Vec

assert_eq!(goal_ctx.total_order(&left, &right), expected);
}

#[test]
fn can_detect_same_key_usage() {
let mut registry = StateKeyRegistry::default();
let same_key = registry.next_key();

let goal_ctx = GoalContextBuilder::with_features(vec![
create_distance_balanced_feature("name_1", None, registry.next_key(), same_key).unwrap(),
create_distance_balanced_feature("name_2", None, registry.next_key(), registry.next_key()).unwrap(),
create_distance_balanced_feature("name_3", None, registry.next_key(), same_key).unwrap(),
]);

match goal_ctx {
Ok(_) => unreachable!(),
Err(message) => {
assert_eq!(message, GenericError::from("feature name_1 and name_3 shares common state keys for caching"))
}
}
}

#[test]
fn can_detect_same_name_usage() {
let goal_ctx = GoalContextBuilder::with_features(vec![
create_objective_feature_with_dynamic_cost("name_1", Arc::new(|_, _| 1.)),
create_objective_feature_with_dynamic_cost("name_2", Arc::new(|_, _| 1.)),
create_objective_feature_with_dynamic_cost("name_1", Arc::new(|_, _| 1.)),
]);

match goal_ctx {
Ok(_) => unreachable!(),
Err(message) => {
assert_eq!(
message,
GenericError::from(
"some of the features are defined more than once, check ids list: name_1,name_2,name_1"
)
)
}
}
}

0 comments on commit 5b39ff3

Please sign in to comment.