Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 0 additions & 6 deletions crates/arco-cli/src/inspect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,15 +272,13 @@ fn build_set_records(program: &SemanticProgram, filtered_set_names: &[&str]) ->
}

fn find_set_alias(program: &SemanticProgram, set_name: &str) -> Option<String> {
// Check variable families for index aliases that map to this set
for family in &program.variable_families {
for (index, domain) in &family.index_domains {
if domain == set_name && index != set_name {
return Some(index.clone());
}
}
}
// Check constraint generation bindings
for constraint in &program.active_constraints {
for binding in &constraint.generation_bindings {
if binding.domain == set_name && binding.variable != set_name {
Expand All @@ -295,11 +293,9 @@ fn infer_set_dtype(resolved: &arco_kdl::semantic::ResolvedSet) -> String {
if resolved.values.is_empty() {
return "string".to_string();
}
// Check if all values are integers
if resolved.values.iter().all(|v| v.parse::<i64>().is_ok()) {
return "int".to_string();
}
// Check if all values are floats
if resolved.values.iter().all(|v| v.parse::<f64>().is_ok()) {
return "float64".to_string();
}
Expand Down Expand Up @@ -893,7 +889,6 @@ fn collect_term_refs_from_expr(
})
.collect();

// Check if this target is already in the output
if !out.iter().any(|r| r.name == *target) {
out.push(TermRef {
name: target.clone(),
Expand Down Expand Up @@ -1064,7 +1059,6 @@ fn build_objective_terms(
});
}
Expr::Reduction(reduction) => {
// Check if the body references named expressions
let body_terms = split_additive_terms(&reduction.body);
let mut found_expressions = false;
for body_term in &body_terms {
Expand Down
3 changes: 0 additions & 3 deletions crates/arco-cli/tests/example_cli_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ fn inspect_produces_valid_toml() {
let stdout = String::from_utf8_lossy(&output.stdout);
let parsed: toml::Value = toml::from_str(&stdout).expect("output should be valid TOML");

// Check top-level sections exist
assert!(parsed.get("meta").is_some(), "should have meta section");
assert!(parsed.get("set").is_some(), "should have set section");
assert!(
Expand Down Expand Up @@ -84,15 +83,13 @@ fn inspect_json_produces_valid_json() {

let payload: Value = serde_json::from_slice(&output.stdout).expect("valid inspect json");

// Check structure matches TOML layout
assert!(payload.get("meta").is_some());
assert!(payload.get("set").is_some());
assert!(payload.get("variable").is_some());
assert!(payload.get("parameter").is_some());
assert!(payload.get("constraint").is_some());
assert!(payload.get("objective").is_some());

// Check counts
let counts = payload["meta"]["counts"]
.as_object()
.expect("counts object");
Expand Down
2 changes: 1 addition & 1 deletion crates/arco-kdl/src/semantic/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ pub enum SemanticError {
#[diagnostic(
code(arco::semantic::unresolved_rule_set_filter_identifier),
help(
"if token is a categorical value, quote it in filter, e.g. `where {{ a == \"north\" }}`"
"if token is a categorical value, quote it in filter, e.g. `filter {{ a == \"north\" }}`"
)
)]
UnresolvedRuleSetFilterIdentifier {
Expand Down
57 changes: 18 additions & 39 deletions crates/arco-kdl/src/source/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,15 +165,15 @@ fn parse_data(node: &KdlNode, context: &ParseContext<'_>) -> Result<DataDecl, So

Ok(DataDecl {
name: first_arg_string(node, 0, context)?,
source: optional_property_string(node, "source", context)?
.or(optional_property_string(node, "from", context)?)
.ok_or_else(|| SourceError::MissingProperty {
source: optional_property_string(node, "source", context)?.ok_or_else(|| {
SourceError::MissingProperty {
node: node.name().value().to_string(),
property: "source",
path: context.path.to_path_buf(),
source_text: Box::new(context.source_text.clone()),
span: node.span(),
})?,
}
})?,
maps,
sets,
indices,
Expand Down Expand Up @@ -210,6 +210,13 @@ fn parse_param(
.transpose()
.map_err(|error| algebra_error(node, error.to_string(), context))?;

for child in node.iter_children() {
match child.name().value() {
"index" | "filter" | "reduce" => {}
other => return Err(unsupported_declaration_error(child, other, context)),
}
}

Ok(crate::source::ParamDecl {
name: first_arg_string(node, 0, context)?,
value: positional_value(node, &indices, context)?,
Expand Down Expand Up @@ -264,7 +271,7 @@ fn parse_set(node: &KdlNode, context: &ParseContext<'_>) -> Result<SetDecl, Sour
domain: Some(domain),
});
}
"filter" | "where" => {
"filter" => {
filter_expression = Some(algebra_text_from_node(child, context)?);
}
member => {
Expand Down Expand Up @@ -562,31 +569,17 @@ data "D" source="file.csv" {
}

#[test]
fn data_from_property_parses_same_as_source() {
fn data_from_property_is_rejected() {
let path = PathBuf::from("test.kdl");

let with_source = r#"
data "D" source="file.csv" {
map "X" from="name"
}
"#;

let with_from = r#"
data "D" from="file.csv" {
map "X" from="name"
}
"#;

let source_parsed = parse_program_text(with_source, &path).expect("source= syntax parses");
let from_parsed = parse_program_text(with_from, &path).expect("from= syntax parses");

assert_eq!(source_parsed.program.data.len(), 1);
assert_eq!(from_parsed.program.data.len(), 1);
assert_eq!(
source_parsed.program.data[0].source,
from_parsed.program.data[0].source
);
assert_eq!(from_parsed.program.data[0].source, "file.csv");
let error = parse_program_text(with_from, &path).expect_err("from= should be rejected");
assert!(error.to_string().contains("source"));
}

#[test]
Expand All @@ -610,30 +603,19 @@ data "D" from="file.csv" {
}

#[test]
fn where_keyword_parses_same_as_filter() {
fn where_keyword_is_rejected() {
let path = PathBuf::from("test.kdl");

let with_where = r#"set "thermal" { in "gen"; where { type == "thermal" } }"#;
let with_filter = r#"set "thermal" { in "gen"; filter { type == "thermal" } }"#;

let where_parsed = parse_program_text(with_where, &path).expect("where syntax parses");
let filter_parsed = parse_program_text(with_filter, &path).expect("filter syntax parses");

assert_eq!(where_parsed.program.sets.len(), 1);
assert_eq!(filter_parsed.program.sets.len(), 1);

assert_eq!(
where_parsed.program.sets[0].filter_expression,
filter_parsed.program.sets[0].filter_expression
);
assert!(where_parsed.program.sets[0].filter_expression.is_some());
parse_program_text(with_where, &path).expect_err("where syntax should fail");
}

#[test]
fn mixed_old_and_new_syntax_parses() {
let path = PathBuf::from("test.kdl");

// Mix of old (control, map, alias=, filter) and new (var, alias, as=, where)
// Mix of legacy-compatible and canonical forms we still support.
let mixed = r#"
data "D" source="file.csv" {
map "old_col" from="x"
Expand All @@ -654,16 +636,13 @@ scenario "S" { use "M" }

let parsed = parse_program_text(mixed, &path).expect("mixed syntax parses");

// Verify data block
assert_eq!(parsed.program.data.len(), 1);
assert_eq!(parsed.program.data[0].maps.len(), 2);

// Verify sets
assert_eq!(parsed.program.sets.len(), 2);
assert_eq!(parsed.program.sets[0].alias, Some("o".to_string()));
assert_eq!(parsed.program.sets[1].alias, Some("n".to_string()));

// Verify model controls (both var and control)
assert_eq!(parsed.program.models.len(), 1);
assert_eq!(parsed.program.models[0].controls.len(), 2);
}
Expand Down
4 changes: 2 additions & 2 deletions crates/arco-kdl/src/source/parser_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub(super) fn parse_optional_filter_expression(
context: &ParseContext<'_>,
) -> Result<Option<String>, SourceError> {
for child in node.iter_children() {
if matches!(child.name().value(), "filter" | "where") {
if child.name().value() == "filter" {
return Ok(Some(algebra_text_from_node(child, context)?));
}
}
Expand Down Expand Up @@ -91,7 +91,7 @@ pub(super) fn declaration_indices(

for child in node.iter_children() {
match child.name().value() {
"filter" | "where" | "bounds" => {}
"filter" | "bounds" => {}
"index" => {
let index_name = first_arg_string(child, 0, context)?;
let domain = child
Expand Down
3 changes: 1 addition & 2 deletions crates/arco-kdl/src/source/surface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ fn rewrite_math_block_at(text: &str, start: usize) -> Option<(String, usize)> {
b'm' => rewrite_math_block(text, start, "minimize")
.or_else(|| rewrite_math_block(text, start, "maximize")),
b'u' => rewrite_math_block(text, start, "upper"),
b'w' => rewrite_math_block(text, start, "where"),
_ => None,
}
}
Expand Down Expand Up @@ -119,7 +118,7 @@ fn rewrite_math_block(text: &str, start: usize, keyword: &str) -> Option<(String
"constraint" => format!("{header} expression={encoded_body}"),
"expression" => format!("{header} {{ formula {encoded_body} }}"),
"minimize" | "maximize" => format!("{header} expression={encoded_body}"),
"lower" | "upper" | "if" | "filter" | "where" => {
"lower" | "upper" | "if" | "filter" => {
format!("{header} expression={encoded_body}")
}
_ => return None,
Expand Down
20 changes: 10 additions & 10 deletions crates/arco-kdl/tests/compile_suite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ data "links" source="data/links.csv" {
index "i" { in "tech" }
index "g" { in "generators" }
index "b" { in "buses" }
where { feasible > 0 }
filter { feasible > 0 }
}
}

Expand Down Expand Up @@ -548,7 +548,7 @@ data "links" source="data/links.csv" {
index "i" { in "tech" }
index "g" { in "generators" }
index "b" { in "buses" }
where { feasible > 0 }
filter { feasible > 0 }
}
}

Expand Down Expand Up @@ -621,7 +621,7 @@ data "links" source="data/links.csv" {
index "i" { in "tech" }
index "g" { in "generators" }
index "b" { in "buses" }
where { feasible > 0 }
filter { feasible > 0 }
}
}

Expand All @@ -630,7 +630,7 @@ set "feasible_links" {
index "i" { in "tech" }
index "g" { in "generators" }
index "b" { in "buses" }
where { a == "1" }
filter { a == "1" }
}

model "TupleDispatch" {
Expand Down Expand Up @@ -698,15 +698,15 @@ data "links" source="data/links.csv" {
index "i" { in "tech" }
index "g" { in "generators" }
index "b" { in "buses" }
where { feasible > 0 }
filter { feasible > 0 }
}
}

set "target_pairs" {
in "feasible_links"
index "a" { in "area" }
index "i" { in "tech" }
where { generators == "g1" }
filter { generators == "g1" }
}

model "TupleDispatch" {
Expand Down Expand Up @@ -782,7 +782,7 @@ data "links" source="data/links.csv" {
index "i" { in "tech" }
index "g" { in "generators" }
index "b" { in "buses" }
where { feasible > 0 }
filter { feasible > 0 }
}
}

Expand Down Expand Up @@ -850,7 +850,7 @@ data "links" source="data/links.csv" {
index "i" { in "tech" }
index "g" { in "generators" }
index "b" { in "buses" }
where { feasible > 0 }
filter { feasible > 0 }
}
}

Expand Down Expand Up @@ -916,7 +916,7 @@ data "links" source="data/links.csv" {
index "i" { in "tech" }
index "g" { in "generators" }
index "b" { in "buses" }
where { feasible > 0 }
filter { feasible > 0 }
}
}

Expand Down Expand Up @@ -1073,7 +1073,7 @@ data "links" source="data/links.csv" {
index "i" { in "tech" }
index "g" { in "generators" }
index "b" { in "buses" }
where { feasible > 0 }
filter { feasible > 0 }
}
}

Expand Down
6 changes: 3 additions & 3 deletions crates/arco-kdl/tests/semantic_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ data "links" source="data/links.csv" {
index "i" { in "tech" }
index "g" { in "generators" }
index "b" { in "buses" }
where { feasible > 0 }
filter { feasible > 0 }
}
}

Expand Down Expand Up @@ -692,7 +692,7 @@ set "tech" { "wind"; "solar" }
set "feasible_links" {
index "a" { in "area" }
index "i" { in "tech" }
where { unknown_col == "1" }
filter { unknown_col == "1" }
}

model "Dispatch" {
Expand Down Expand Up @@ -894,7 +894,7 @@ data "links" source="data/links.csv" {
index "i" { in "tech" }
index "g" { in "generators" }
index "b" { in "buses" }
where { feasible > 0 }
filter { feasible > 0 }
}
}

Expand Down
6 changes: 3 additions & 3 deletions docs/appendix-a-ergonomic-profile.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ canonical `set { in ...; filter { ... } }` syntax defined in
This is a usage guide, not new grammar.

```kdl
data branch_data from="data/branches.csv" {
data branch_data source="data/branches.csv" {
set edge
set active_edge { in edge; filter { conex == 1 } }
param conex index=edge
Expand Down Expand Up @@ -82,7 +82,7 @@ data branch_data from="data/branches.csv" {
> index i { in tech }
> index g { in generators }
> index b { in buses }
> where { feasible > 0 }
> filter { feasible > 0 }
> }
> }
>
Expand All @@ -92,7 +92,7 @@ data branch_data from="data/branches.csv" {
> index i { in tech }
> index g { in generators }
> index b { in buses }
> where { area == "south" }
> filter { area == "south" }
> }
>
> model nodal_allocation {
Expand Down
Loading
Loading