StudioInfinity/dyna3
2
0
Fork 1
Code Issues 56 Pull requests 1 Projects 3 Releases 4 Wiki Activity Actions

Curvature regulators #80

Merged
glen merged 21 commits from Vectornaut/dyna3:curvature-regulators into main 2025-04-21 23:40:43 +00:00
Conversation 22 Commits 21 Files changed 6 +75 -35
3 changed files with 75 additions and 35 deletions
Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit 126d4c0cce - Show all commits

2
app-proto/src/add_remove.rs
View file

@ -199,7 +199,7 @@ pub fn AddRemove() -> View {
.try_into()
.unwrap()
);
state.assembly.insert_new_regulator(subjects);
state.assembly.insert_new_product_regulator(subjects);
state.selection.update(|sel| sel.clear());
}
) { "🔗" }

72
app-proto/src/assembly.rs
View file

@ -1,7 +1,7 @@
use nalgebra::{DMatrix, DVector, DVectorView, Vector3};
use rustc_hash::FxHashMap;
use slab::Slab;
use std::{collections::BTreeSet, sync::atomic::{AtomicU64, Ordering}};
use std::{collections::BTreeSet, rc::Rc, sync::atomic::{AtomicU64, Ordering}};
use sycamore::prelude::*;
use web_sys::{console, wasm_bindgen::JsValue}; /* DEBUG */
@ -132,14 +132,35 @@ impl Element {
}
}
#[derive(Clone, Copy)]
pub trait Regulator {
// get information
fn subjects(&self) -> Vec<ElementKey>;
glen marked this conversation as resolved Outdated
glen commented 2025-04-21 19:45:36 +00:00
Outdated
Copy link

The check here and the potential panic if it fails (and analogous code in two other ProblemPoser implementations) is evidence of misfactoring: in fact, the ProblemPosers only ever get to execute when every element has just been assigned a column index. So there is no need to have such a check.

I am open to any reasonable way to factor these checks out. One is to insist that all elements in an assembly always have a column index. As far as I can see, the only thing that None column indices are currently used for is a transient period between an Element being added to an Assembly and the next call to that Assembly being realized. That period could be eliminated by simply assigning each Element the next available column index and immediately calling realize. Or there could be a method (likely in engine?) that takes a realized assembly and a new completely unconstrained element and updates the realization to include the unconstrained element -- that should presumably be far less work than a full call to realize. As far as I can tell, the only consistency bit that needs to be kept up is the tangent space bases in the realization. But I suppose it should be possible to calculate the new tangent space basis matriceso directly in the case of a new unconstrained element added to a realized assembly.

Or we can insist that elements always have an index by assigning indices on creation, and dropping the invariant that an element with an index has to have tangent motions in the corresponding column of the tangent space basis matrices. That might not be so terrible -- if an element's column is out of range for the tangent space basis matrices, that must mean it is totally unconstrained, and so we can actually deform it freely, and so just switch to simpler deformation code for that case.

Another possibility is to initially use column index -1 for a new element, and not worry about any -1s in the problem posing because the code is structured such that they never occur when a problem is being posed; we only need to worry about the -1s when deforming, and handle the case there.

Another possibility (if I understand Rust and the Option type properly) is to simply use unwrap in these posers. It nominally could panic, but we know it won't because problems are only posed just after every element has been assigned an index. This route seems the least satisfying because we still have all of the baggage of Option even though the other possibilities make it clear that Option isn't filling a particularly critical need for the column indices of elements. But this route also might be the path of least resistance, so I'm OK if that's the way you want to go here.

Or there might be some other mechanism whereby the data structure passed to a problem poser is something of a slightly different type that definitely has a column for each element. (Perhaps this mild redundant-checking issue is even evidence that elements shouldn't store their column indices; conceptually, a column index is more of a property of an Element as a part of an Assembly, rather than intrinsically of the Element itself. So maybe instead the Assembly has some record of the column index for each Element, where the indices are plain ol' integers, and that record is provided to problem posers. But I am definitely not mandating a refactor of that scope as part of this PR.)

What doesn't seem good is writing out checks and manual panic messages in three places, for panics that can literally never occur because of the structure of the code. That redundant checking is what I mean by "evidence of misfactoring".

The check here and the potential panic if it fails (and analogous code in two other ProblemPoser implementations) is evidence of misfactoring: in fact, the ProblemPosers only ever get to execute when every element has just been assigned a column index. So there is no need to have such a check. I am open to any reasonable way to factor these checks out. One is to insist that all elements in an assembly always have a column index. As far as I can see, the only thing that None column indices are currently used for is a transient period between an Element being added to an Assembly and the next call to that Assembly being realized. That period could be eliminated by simply assigning each Element the next available column index and immediately calling realize. Or there could be a method (likely in engine?) that takes a realized assembly and a new completely unconstrained element and updates the realization to include the unconstrained element -- that should presumably be far less work than a full call to realize. As far as I can tell, the only consistency bit that needs to be kept up is the tangent space bases in the realization. But I suppose it should be possible to calculate the new tangent space basis matriceso directly in the case of a new unconstrained element added to a realized assembly. Or we can insist that elements always have an index by assigning indices on creation, and dropping the invariant that an element with an index has to have tangent motions in the corresponding column of the tangent space basis matrices. That might not be so terrible -- if an element's column is out of range for the tangent space basis matrices, that must mean it is totally unconstrained, and so we can actually deform it freely, and so just switch to simpler deformation code for that case. Another possibility is to initially use column index -1 for a new element, and not worry about any -1s in the problem posing because the code is structured such that they never occur when a problem is being posed; we only need to worry about the -1s when deforming, and handle the case there. Another possibility (if I understand Rust and the Option type properly) is to simply use unwrap in these posers. It nominally could panic, but we know it won't because problems are only posed just after every element has been assigned an index. This route seems the least satisfying because we still have all of the baggage of Option even though the other possibilities make it clear that Option isn't filling a particularly critical need for the column indices of elements. But this route also might be the path of least resistance, so I'm OK if that's the way you want to go here. Or there might be some other mechanism whereby the data structure passed to a problem poser is something of a slightly different type that definitely has a column for each element. (Perhaps this mild redundant-checking issue is even evidence that elements shouldn't store their column indices; conceptually, a column index is more of a property of an Element as a part of an Assembly, rather than intrinsically of the Element itself. So maybe instead the Assembly has some record of the column index for each Element, where the indices are plain ol' integers, and that record is provided to problem posers. But I am definitely not mandating a refactor of that scope as part of this PR.) What doesn't seem good is writing out checks and manual panic messages in three places, for panics that can literally never occur because of the structure of the code. That redundant checking is what I mean by "evidence of misfactoring".
Vectornaut commented 2025-04-21 21:56:27 +00:00
Outdated
Copy link

I think there are two problems here—a narrow one that falls within the scope of this pull request, and a broader one that deserves its own pull request.

Narrow: semantics issue

The conditional panic doesn't do a great job of communicating that we expect every element to have a column index when pose is called. Unwrapping the Option, as you point out, has the semantics we want, and that's what we do on the main branch (commit b86f176). However, unwrapping doesn't provide an informative panic message, which would make it harder to debug an invariant violation.

I've switched to the expect method, which has the same semantics, and allows us to provide our own panic message (commit 5eeb093). I've also rewritten the panic messages in the style recommended by the expect documentation.

Broad: factoring issue

I agree that we should eventually rewrite the column index system to avoid these checks entirely. I think that deserves its own pull request, since this pull request didn't create and doesn't modify the column index system.

I think there are two problems here—a narrow one that falls within the scope of this pull request, and a broader one that deserves its own pull request. #### Narrow: semantics issue The conditional panic doesn't do a great job of communicating that we expect every element to have a column index when `pose` is called. Unwrapping the `Option`, as you point out, has the semantics we want, and that's what we do on the main branch (commit b86f176). However, unwrapping doesn't provide an informative panic message, which would make it harder to debug an invariant violation. I've switched to the [`expect`](https://doc.rust-lang.org/std/option/enum.Option.html#method.expect) method, which has the same semantics, and allows us to provide our own panic message (commit 5eeb093). I've also rewritten the panic messages in the [style recommended](https://doc.rust-lang.org/std/option/enum.Option.html#recommended-message-style) by the `expect` documentation. #### Broad: factoring issue I agree that we should eventually rewrite the column index system to avoid these checks entirely. I think that deserves its own pull request, since this pull request didn't create and doesn't modify the column index system.
fn measurement(&self) -> ReadSignal<f64>;
fn set_point(&self) -> Signal<SpecifiedValue>;
// write problem data
fn write_to_problem(&self, problem: &mut ConstraintProblem, elts: &Slab<Element>);
}
pub struct ProductRegulator {
pub subjects: [ElementKey; 2],
pub measurement: ReadSignal<f64>,
pub set_point: Signal<SpecifiedValue>
}
impl ProductRegulator {
impl Regulator for ProductRegulator {
fn subjects(&self) -> Vec<ElementKey> {
self.subjects.into()
}
fn measurement(&self) -> ReadSignal<f64> {
self.measurement
}
fn set_point(&self) -> Signal<SpecifiedValue> {
self.set_point
}
fn write_to_problem(&self, problem: &mut ConstraintProblem, elts: &Slab<Element>) {
self.set_point.with_untracked(|set_pt| {
if let Some(val) = set_pt.value {
@ -169,7 +190,7 @@ type AssemblyMotion<'a> = Vec<ElementMotion<'a>>;
pub struct Assembly {
// elements and regulators
pub elements: Signal<Slab<Element>>,
pub regulators: Signal<Slab<ProductRegulator>>,
pub regulators: Signal<Slab<Rc<dyn Regulator>>>,
// solution variety tangent space. the basis vectors are stored in
// configuration matrix format, ordered according to the elements' column
@ -240,19 +261,23 @@ impl Assembly {
);
}
fn insert_regulator(&self, regulator: ProductRegulator) {
let subjects = regulator.subjects;
let key = self.regulators.update(|regs| regs.insert(regulator));
let subject_regulators = self.elements.with(
|elts| subjects.map(|subj| elts[subj].regulators)
fn insert_regulator(&self, regulator: Rc<dyn Regulator>) {
let subjects = regulator.subjects();
let key = self.regulators.update(
|regs| regs.insert(regulator)
);
let subject_regulators: Vec<_> = self.elements.with(
|elts| subjects.into_iter().map(
|subj| elts[subj].regulators
).collect()
);
for regulators in subject_regulators {
regulators.update(|regs| regs.insert(key));
}
}
pub fn insert_new_regulator(self, subjects: [ElementKey; 2]) {
// create and insert a new regulator
pub fn insert_new_product_regulator(self, subjects: [ElementKey; 2]) {
// create and insert a new product regulator
let measurement = self.elements.map(
move |elts| {
let representations = subjects.map(|subj| elts[subj].representation);
@ -264,26 +289,31 @@ impl Assembly {
}
);
let set_point = create_signal(SpecifiedValue::from_empty_spec());
self.insert_regulator(ProductRegulator {
self.insert_regulator(Rc::new(ProductRegulator {
subjects: subjects,
measurement: measurement,
set_point: set_point
});
}));
/* DEBUG */
// print an updated list of regulators
console::log_1(&JsValue::from("Regulators:"));
self.regulators.with(|regs| {
for (_, reg) in regs.into_iter() {
console::log_5(
&JsValue::from(" "),
&JsValue::from(reg.subjects[0]),
&JsValue::from(reg.subjects[1]),
&JsValue::from(":"),
&reg.set_point.with_untracked(
|set_pt| JsValue::from(set_pt.spec.as_str())
console::log_1(&JsValue::from(format!(
" {:?}: {}",
reg.subjects(),
reg.set_point().with_untracked(
|set_pt| {
let spec = &set_pt.spec;
if spec.is_empty() {
"__".to_string()
} else {
spec.clone()
}
}
)
);
)));
}
});

36
app-proto/src/outline.rs
View file

@ -1,4 +1,5 @@
use itertools::Itertools;
use std::rc::Rc;
use sycamore::prelude::*;
use web_sys::{
KeyboardEvent,
@ -9,24 +10,32 @@ use web_sys::{
use crate::{
AppState,
assembly,
assembly::{ElementKey, ProductRegulator, RegulatorKey},
assembly::{ElementKey, Regulator, RegulatorKey},
specified::SpecifiedValue
};
// an editable view of a regulator
#[component(inline_props)]
fn RegulatorInput(regulator: ProductRegulator) -> View {
fn RegulatorInput(regulator: Rc<dyn Regulator>) -> View {
// get the regulator's measurement and set point signals
let measurement = regulator.measurement();
let set_point = regulator.set_point();
// the `valid` signal tracks whether the last entered value is a valid set
// point specification
let valid = create_signal(true);
// the `value` signal holds the current set point specification
let value = create_signal(
regulator.set_point.with_untracked(|set_pt| set_pt.spec.clone())
set_point.with_untracked(|set_pt| set_pt.spec.clone())
);
// this closure resets the input value to the regulator's set point
// specification
// this `reset_value` closure resets the input value to the regulator's set
// point specification
let reset_value = move || {
batch(|| {
valid.set(true);
value.set(regulator.set_point.with(|set_pt| set_pt.spec.clone()));
value.set(set_point.with(|set_pt| set_pt.spec.clone()));
})
};
@ -39,7 +48,7 @@ fn RegulatorInput(regulator: ProductRegulator) -> View {
r#type="text",
class=move || {
if valid.get() {
regulator.set_point.with(|set_pt| {
set_point.with(|set_pt| {
if set_pt.is_present() {
"regulator-input constraint"
} else {
@ -50,13 +59,13 @@ fn RegulatorInput(regulator: ProductRegulator) -> View {
"regulator-input invalid"
}
},
placeholder=regulator.measurement.with(|result| result.to_string()),
placeholder=measurement.with(|result| result.to_string()),
bind:value=value,
on:change=move |_| {
valid.set(
match SpecifiedValue::try_from(value.get_clone_untracked()) {
Ok(set_pt) => {
regulator.set_point.set(set_pt);
set_point.set(set_pt);
true
}
Err(_) => false
@ -80,11 +89,12 @@ fn RegulatorInput(regulator: ProductRegulator) -> View {
fn RegulatorOutlineItem(regulator_key: RegulatorKey, element_key: ElementKey) -> View {
let state = use_context::<AppState>();
let assembly = &state.assembly;
let regulator = assembly.regulators.with(|regs| regs[regulator_key]);
let other_subject = if regulator.subjects[0] == element_key {
regulator.subjects[1]
let regulator = assembly.regulators.with(|regs| regs[regulator_key].clone());
let subjects = regulator.subjects();
let other_subject = if subjects[0] == element_key {
subjects[1]
} else {
regulator.subjects[0]
subjects[0]
};
let other_subject_label = assembly.elements.with(|elts| elts[other_subject].label.clone());
view! {