forked from 1b-agent/fieldcraft-upstream
engine: refuse unknown command parameters, suggest a valid name, document heat loads and thermal materials
Every command with a schema now refuses a top-level key it does not take, naming the key and the accepted ones (#316). A bad definition name's error suggests the nearest valid one (#320). The load and material params docs list the heat loads and the thermal block. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01UhhZMBTviDQxZvwnyP89fo
This commit is contained in:
1 parent
35ee88aa18
commit
7dd4043afe
4 files changed
+107
-4
No files matched your search
@@ -92,7 +92,7 @@ definition_commands!(
|
|||||||
Material,
|
Material,
|
||||||
"material",
|
"material",
|
||||||
"Material",
|
"Material",
|
||||||
r#"{"name": "Steel", "elastic": {"type": "isotropic", "young": 210000, "poisson": 0.3} | {"type": "engineering_constants", "young": [E1, E2, E3], "poisson": [ν12, ν13, ν23], "shear": [G12, G13, G23]}, "density": 7.85e-9, "fluid": {"type": "newtonian", "viscosity": 1.0e-9} (dynamic viscosity; a fluid needs a density), "plastic": {…} (see model.material.plasticity), "damage": {…}, "erosion": {…} (see model.material.damage), "hyperelastic": {"type": "neo_hookean"} | {"type": "mooney_rivlin", "c01": 0.1} (C10 = μ/2 − C01) | {"type": "yeoh", "c20": -0.01, "c30": 0.001} (C10 = μ/2) (large strain in nonlinear steps, from the isotropic constants: μ = E/(2(1 + ν)), κ = E/(3(1 − 2ν)); without it an elastic material is St Venant–Kirchhoff there)} — in the model's unit system (model.units), never converted"#,
|
r#"{"name": "Steel", "elastic": {"type": "isotropic", "young": 210000, "poisson": 0.3} | {"type": "engineering_constants", "young": [E1, E2, E3], "poisson": [ν12, ν13, ν23], "shear": [G12, G13, G23]}, "density": 7.85e-9, "fluid": {"type": "newtonian", "viscosity": 1.0e-9} (dynamic viscosity; a fluid needs a density), "plastic": {…} (see model.material.plasticity), "damage": {…}, "erosion": {…} (see model.material.damage), "hyperelastic": {"type": "neo_hookean"} | {"type": "mooney_rivlin", "c01": 0.1} (C10 = μ/2 − C01) | {"type": "yeoh", "c20": -0.01, "c30": 0.001} (C10 = μ/2) (large strain in nonlinear steps, from the isotropic constants: μ = E/(2(1 + ν)), κ = E/(3(1 − 2ν)); without it an elastic material is St Venant–Kirchhoff there), "thermal": {"conductivity": 0.05 | [kx, ky, kz], "specific_heat": 4.6e8, "expansion": 1.2e-5 | [αx, αy, αz], "reference_temperature": 20 (zero thermal strain; default 0)} (heat transfer and thermal strain; each key optional)} — in the model's unit system (model.units), never converted"#,
|
||||||
super::forms::MATERIAL,
|
super::forms::MATERIAL,
|
||||||
super::forms::MATERIAL_DELETE,
|
super::forms::MATERIAL_DELETE,
|
||||||
add_material,
|
add_material,
|
||||||
@@ -267,7 +267,7 @@ definition_commands!(
|
|||||||
Load,
|
Load,
|
||||||
"load",
|
"load",
|
||||||
"Load",
|
"Load",
|
||||||
r#"{"name": "push", "type": "force|moment", "target": {"set": "…"} | {"reference_point": "…"}, "vector": [x, y, z], "distribution": "total" (default: the vector is the total, split by area over the set's faces, or equally over its nodes) | "per_node" (at every node: depends on the mesh)} | {"name", "type": "traction", "set", "vector" (per area, on the set's faces)} | {"name", "type": "pressure", "set", "magnitude" (positive pushes in)} | {"name", "type": "gravity", "acceleration": [x, y, z], "set" (optional, default the whole model)} | {"name", "type": "body_force", "set", "vector" (per volume)} | {"name", "type": "centrifugal", "angular_velocity" (rad per time unit), "point": [x, y, z] (on the axis), "axis": [x, y, z], "set" (optional)} — 2D models: in-plane vectors, surface loads on element edges; active in the steps that list it (model.step.activate)"#,
|
r#"{"name": "push", "type": "force|moment", "target": {"set": "…"} | {"reference_point": "…"}, "vector": [x, y, z], "distribution": "total" (default: the vector is the total, split by area over the set's faces, or equally over its nodes) | "per_node" (at every node: depends on the mesh)} | {"name", "type": "traction", "set", "vector" (per area, on the set's faces)} | {"name", "type": "pressure", "set", "magnitude" (positive pushes in)} | {"name", "type": "gravity", "acceleration": [x, y, z], "set" (optional, default the whole model)} | {"name", "type": "body_force", "set", "vector" (per volume)} | {"name", "type": "centrifugal", "angular_velocity" (rad per time unit), "point": [x, y, z] (on the axis), "axis": [x, y, z], "set" (optional)} | {"name", "type": "surface_flux", "set", "flux" (heat per area into the set's faces)} | {"name", "type": "convection", "set", "film" (film coefficient h), "sink" (surrounding temperature): heat flux h (sink − T) into the set's faces} | {"name", "type": "heat_source", "set", "power" (heat per volume in the set's elements)} | {"name", "type": "temperature", "target": {"set": "…"} | {"reference_point": "…"}, "value" (a temperature that strains the material in stress steps)} — heat loads act in heat transfer and coupled steps; 2D models: in-plane vectors, surface loads on element edges; active in the steps that list it (model.step.activate)"#,
|
||||||
super::forms::LOAD,
|
super::forms::LOAD,
|
||||||
super::forms::LOAD_DELETE,
|
super::forms::LOAD_DELETE,
|
||||||
add_load,
|
add_load,
|
||||||
|
|||||||
@@ -171,6 +171,16 @@ pub fn find_command(id: &str) -> Option<&'static CommandSpec> {
|
|||||||
commands().iter().find(|c| c.id == id)
|
commands().iter().find(|c| c.id == id)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Refuse a parameter the command does not take, naming the ones it does: a script that sets
|
||||||
|
/// `deformed` on `results.contour` learns it belongs elsewhere instead of being ignored.
|
||||||
|
fn check_keys(spec: &CommandSpec, params: &Value) -> Result<()> {
|
||||||
|
match spec.form.and_then(|form| schema::unknown_key(form, params)) {
|
||||||
|
Some((key, accepted)) if accepted.is_empty() => Err(EngineError::Params(format!("{} takes no parameters (got `{key}`)", spec.id))),
|
||||||
|
Some((key, accepted)) => Err(EngineError::Params(format!("{} has no parameter `{key}`; it takes {}", spec.id, accepted.join(", ")))),
|
||||||
|
None => Ok(()),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// A new revision number, unique in the process: every state of every session has its own.
|
/// A new revision number, unique in the process: every state of every session has its own.
|
||||||
pub(crate) fn next_revision() -> u64 {
|
pub(crate) fn next_revision() -> u64 {
|
||||||
static NEXT: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1);
|
static NEXT: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1);
|
||||||
@@ -269,7 +279,11 @@ impl Session {
|
|||||||
let spec = find_command(id).ok_or_else(|| EngineError::UnknownCommand(id.chars().take(100).collect()))?;
|
let spec = find_command(id).ok_or_else(|| EngineError::UnknownCommand(id.chars().take(100).collect()))?;
|
||||||
let timer = usage::Timer::start(self.origin);
|
let timer = usage::Timer::start(self.origin);
|
||||||
let elements = self.model.elements().len();
|
let elements = self.model.elements().len();
|
||||||
let result = self.availability(spec).map_err(EngineError::Unavailable).and_then(|()| self.run_command(spec, params));
|
let result = self
|
||||||
|
.availability(spec)
|
||||||
|
.map_err(EngineError::Unavailable)
|
||||||
|
.and_then(|()| check_keys(spec, params))
|
||||||
|
.and_then(|()| self.run_command(spec, params));
|
||||||
if timer.counting() {
|
if timer.counting() {
|
||||||
let milestone = usage::milestone(spec.id, (elements, self.model.elements().len()));
|
let milestone = usage::milestone(spec.id, (elements, self.model.elements().len()));
|
||||||
let sample =
|
let sample =
|
||||||
|
|||||||
@@ -777,6 +777,38 @@ pub fn object_problems(fields: &'static [Field], obj: &Map<String, Value>) -> Ve
|
|||||||
out
|
out
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The top-level keys `params` may hold for these fields: each field's key, and for a flattened
|
||||||
|
/// one-of (a load's `type`) its tag and the fields of the variant `params` picks (of every
|
||||||
|
/// variant when it picks none).
|
||||||
|
pub fn accepted_keys(fields: &[Field], params: &Value) -> Vec<&'static str> {
|
||||||
|
let mut keys = Vec::new();
|
||||||
|
for f in fields {
|
||||||
|
match f.kind {
|
||||||
|
Kind::Variants { tag, variants } if f.key.is_empty() => {
|
||||||
|
keys.push(tag);
|
||||||
|
let picked = params.get(tag).and_then(Value::as_str).and_then(|t| variants.iter().find(|v| v.tag == t));
|
||||||
|
for v in variants.iter().filter(|v| picked.is_none_or(|p| p.tag == v.tag)) {
|
||||||
|
for key in accepted_keys(v.fields, params) {
|
||||||
|
if !keys.contains(&key) {
|
||||||
|
keys.push(key);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
_ => keys.push(f.key),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
keys
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The first top-level key of `params` that `fields` do not describe, with the keys they do:
|
||||||
|
/// a misspelt or misplaced parameter is refused rather than silently ignored.
|
||||||
|
pub fn unknown_key(fields: &[Field], params: &Value) -> Option<(String, Vec<&'static str>)> {
|
||||||
|
let obj = params.as_object()?;
|
||||||
|
let accepted = accepted_keys(fields, params);
|
||||||
|
obj.keys().find(|k| !accepted.contains(&k.as_str())).map(|k| (k.chars().take(64).collect(), accepted))
|
||||||
|
}
|
||||||
|
|
||||||
/// The path of field `key` inside `prefix` (`elastic/young`).
|
/// The path of field `key` inside `prefix` (`elastic/young`).
|
||||||
pub fn join(prefix: &str, key: &str) -> String {
|
pub fn join(prefix: &str, key: &str) -> String {
|
||||||
if prefix.is_empty() {
|
if prefix.is_empty() {
|
||||||
@@ -1106,4 +1138,27 @@ mod tests {
|
|||||||
assert_eq!(s["oneOf"][1]["properties"]["type"]["const"], "pressure");
|
assert_eq!(s["oneOf"][1]["properties"]["type"]["const"], "pressure");
|
||||||
assert_eq!(s["oneOf"][1]["required"], json!(["magnitude", "set", "type"]));
|
assert_eq!(s["oneOf"][1]["required"], json!(["magnitude", "set", "type"]));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn unknown_keys_are_found_per_variant() {
|
||||||
|
assert_eq!(unknown_key(LOADISH, &json!({"name": "P", "type": "pressure", "magnitude": 1})), None);
|
||||||
|
let (key, accepted) = unknown_key(LOADISH, &json!({"name": "P", "type": "pressure", "magnitud": 1})).unwrap();
|
||||||
|
assert_eq!(key, "magnitud");
|
||||||
|
assert!(accepted.contains(&"magnitude") && accepted.contains(&"name"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn every_command_refuses_an_unknown_parameter() {
|
||||||
|
for spec in crate::commands() {
|
||||||
|
let mut s = Session::default();
|
||||||
|
let e = s.execute(spec.id, &json!({"bogus_param": 1})).unwrap_err().to_string();
|
||||||
|
// A command that cannot run yet says so first; otherwise the key is named.
|
||||||
|
assert!(e.contains("bogus_param") || s.availability(spec).is_err(), "{}: {e}", spec.id);
|
||||||
|
}
|
||||||
|
let mut s = Session::default();
|
||||||
|
crate::cmd::solve::tests::cantilever(&mut s);
|
||||||
|
s.execute("solve.run", &json!({})).unwrap();
|
||||||
|
let e = s.execute("results.contour", &json!({"field": "displacement", "deformed": true})).unwrap_err().to_string();
|
||||||
|
assert!(e.contains("`deformed`") && e.contains("field"), "{e}");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
+35
-1
@@ -294,10 +294,30 @@ pub fn check_name(what: &str, name: &str) -> Result<()> {
|
|||||||
Ok(())
|
Ok(())
|
||||||
} else {
|
} else {
|
||||||
let shown: String = name.chars().take(MAX_NAME_LEN).collect();
|
let shown: String = name.chars().take(MAX_NAME_LEN).collect();
|
||||||
Err(Error::Invalid(format!("{what} name `{shown}` must be 1 to {MAX_NAME_LEN} characters: a letter, then letters, digits, `_` or `-`")))
|
let rule = format!("{what} name `{shown}` must be 1 to {MAX_NAME_LEN} characters: a letter, then letters, digits, `_` or `-`");
|
||||||
|
Err(Error::Invalid(match suggest_name(name) {
|
||||||
|
Some(fixed) => format!("{rule} (try `{fixed}`)"),
|
||||||
|
None => rule,
|
||||||
|
}))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The valid name closest to `name`: spaces and other characters become `_`, a leading
|
||||||
|
/// non-letter gets an `N` before it, and it is cut to [`MAX_NAME_LEN`]. `None` when nothing of it
|
||||||
|
/// is usable (empty, or no letter or digit at all).
|
||||||
|
pub fn suggest_name(name: &str) -> Option<String> {
|
||||||
|
let trimmed = name.trim();
|
||||||
|
if !trimmed.chars().any(|c| c.is_ascii_alphanumeric()) {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
let mut fixed: String = trimmed.chars().map(|c| if c.is_ascii_alphanumeric() || c == '-' { c } else { '_' }).collect();
|
||||||
|
if !fixed.starts_with(|c: char| c.is_ascii_alphabetic()) {
|
||||||
|
fixed.insert(0, 'N');
|
||||||
|
}
|
||||||
|
fixed.truncate(MAX_NAME_LEN);
|
||||||
|
Some(fixed)
|
||||||
|
}
|
||||||
|
|
||||||
/// Check that a value is a finite number, naming it in the error.
|
/// Check that a value is a finite number, naming it in the error.
|
||||||
pub(crate) fn finite(what: &str, v: f64) -> Result<f64> {
|
pub(crate) fn finite(what: &str, v: f64) -> Result<f64> {
|
||||||
if v.is_finite() { Ok(v) } else { Err(Error::Invalid(format!("{what} must be a finite number, got {v}"))) }
|
if v.is_finite() { Ok(v) } else { Err(Error::Invalid(format!("{what} must be a finite number, got {v}"))) }
|
||||||
@@ -412,3 +432,17 @@ impl Model {
|
|||||||
]
|
]
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod name_tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_bad_name_suggests_a_fixed_one() {
|
||||||
|
let e = check_name("load", "Hot air").unwrap_err().to_string();
|
||||||
|
assert!(e.contains("try `Hot_air`"), "{e}");
|
||||||
|
assert_eq!(suggest_name("1st load"), Some("N1st_load".into()));
|
||||||
|
assert_eq!(suggest_name(" "), None);
|
||||||
|
assert!(check_name("load", &suggest_name(&"x ".repeat(80)).unwrap()).is_ok());
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in new issue
Block a user