From c73008d70235c72bec89f40078b850040c1a7d1a Mon Sep 17 00:00:00 2001 From: Aaron Fenyes Date: Thu, 24 Jul 2025 14:59:21 -0700 Subject: [PATCH 1/6] Trigger realization more directly Simplify the system that reactively triggers realizations, at the cost of removing the preconditioning step described in issue #101 and doing unnecessary realizations after certain kinds of updates. The new system should trigger a realization after any update that could affect the assembly's deformation space. For simplicity, any update to the regulator list triggers an update, even if it doesn't affect the set of constraints. In particular, adding a regulator triggers an unnecessary realization. --- app-proto/src/assembly.rs | 72 +++++-------------- app-proto/src/components/add_remove.rs | 7 +- .../src/components/test_assembly_chooser.rs | 6 -- app-proto/src/engine.rs | 47 ++++-------- 4 files changed, 36 insertions(+), 96 deletions(-) diff --git a/app-proto/src/assembly.rs b/app-proto/src/assembly.rs index 68fcd8b..26fb4aa 100644 --- a/app-proto/src/assembly.rs +++ b/app-proto/src/assembly.rs @@ -16,7 +16,6 @@ use crate::{ components::{display::DisplayItem, outline::OutlineItem}, engine::{ Q, - change_half_curvature, local_unif_to_std, point, project_point_to_normalized, @@ -358,16 +357,6 @@ pub trait Regulator: Serial + ProblemPoser + OutlineItem { fn subjects(&self) -> Vec>; fn measurement(&self) -> ReadSignal; fn set_point(&self) -> Signal; - - // this method is used to responsively precondition the assembly for - // realization when the regulator becomes a constraint, or is edited while - // acting as a constraint. it should track the set point, do any desired - // preconditioning when the set point is present, and use its return value - // to report whether the set is present. the default implementation does no - // preconditioning - fn try_activate(&self) -> bool { - self.set_point().with(|set_pt| set_pt.is_present()) - } } impl Hash for dyn Regulator { @@ -488,18 +477,6 @@ impl Regulator for HalfCurvatureRegulator { fn set_point(&self) -> Signal { self.set_point } - - fn try_activate(&self) -> bool { - match self.set_point.with(|set_pt| set_pt.value) { - Some(half_curv) => { - self.subject.representation().update( - |rep| change_half_curvature(rep, half_curv) - ); - true - } - None => false - } - } } impl Serial for HalfCurvatureRegulator { @@ -552,8 +529,7 @@ pub struct Assembly { pub elements_by_id: Signal>>, // realization control - pub keep_realized: Signal, - pub needs_realization: Signal, + pub realization_trigger: Signal<()>, // realization diagnostics pub realization_status: Signal>, @@ -568,21 +544,23 @@ impl Assembly { regulators: create_signal(BTreeSet::new()), tangent: create_signal(ConfigSubspace::zero(0)), elements_by_id: create_signal(BTreeMap::default()), - keep_realized: create_signal(true), - needs_realization: create_signal(false), + realization_trigger: create_signal(()), realization_status: create_signal(Ok(())), descent_history: create_signal(DescentHistory::new()) }; - // realize the assembly whenever it becomes simultaneously true that - // we're trying to keep it realized and it needs realization + // realize the assembly whenever the element list, the regulator list, + // a regulator's set point, or the realization trigger is updated let assembly_for_effect = assembly.clone(); create_effect(move || { - let should_realize = assembly_for_effect.keep_realized.get() - && assembly_for_effect.needs_realization.get(); - if should_realize { - assembly_for_effect.realize(); - } + assembly_for_effect.elements.track(); + assembly_for_effect.regulators.with( + |regs| for reg in regs { + reg.set_point().track(); + } + ); + assembly_for_effect.realization_trigger.track(); + assembly_for_effect.realize(); }); assembly @@ -646,19 +624,6 @@ impl Assembly { regulators.update(|regs| regs.insert(regulator.clone())); } - // request a realization when the regulator becomes a constraint, or is - // edited while acting as a constraint - let self_for_effect = self.clone(); - create_effect(move || { - /* DEBUG */ - // log the regulator update - console_log!("Updated regulator with subjects {:?}", regulator.subjects()); - - if regulator.try_activate() { - self_for_effect.needs_realization.set(true); - } - }); - /* DEBUG */ // print an updated list of regulators console_log!("Regulators:"); @@ -726,8 +691,10 @@ impl Assembly { } else { console_log!("✅️ Target accuracy achieved!"); } - console_log!("Steps: {}", history.scaled_loss.len() - 1); - console_log!("Loss: {}", history.scaled_loss.last().unwrap()); + if history.scaled_loss.len() > 0 { + console_log!("Steps: {}", history.scaled_loss.len() - 1); + console_log!("Loss: {}", history.scaled_loss.last().unwrap()); + } // report the loss history self.descent_history.set(history); @@ -750,9 +717,6 @@ impl Assembly { // save the tangent space self.tangent.set_silent(tangent); - - // clear the realization request flag - self.needs_realization.set(false); }, Err(message) => { // report the realization status. the `Err(message)` we're @@ -848,10 +812,10 @@ impl Assembly { }); } - // request a realization to bring the configuration back onto the + // trigger a realization to bring the configuration back onto the // solution variety. this also gets the elements' column indices and the // saved tangent space back in sync - self.needs_realization.set(true); + self.realization_trigger.set(()); } } diff --git a/app-proto/src/components/add_remove.rs b/app-proto/src/components/add_remove.rs index 3b0f9e0..ef5851c 100644 --- a/app-proto/src/components/add_remove.rs +++ b/app-proto/src/components/add_remove.rs @@ -14,7 +14,12 @@ pub fn AddRemove() -> View { button( on:click=|_| { let state = use_context::(); - state.assembly.insert_element_default::(); + batch(|| { + // this call is batched to avoid redundant realizations. + // it updates the element list and the regulator list, + // which are both tracked by the realization effect + state.assembly.insert_element_default::(); + }); } ) { "Add sphere" } button( diff --git a/app-proto/src/components/test_assembly_chooser.rs b/app-proto/src/components/test_assembly_chooser.rs index 232cda3..b58dd1a 100644 --- a/app-proto/src/components/test_assembly_chooser.rs +++ b/app-proto/src/components/test_assembly_chooser.rs @@ -900,9 +900,6 @@ pub fn TestAssemblyChooser() -> View { let state = use_context::(); let assembly = &state.assembly; - // pause realization - assembly.keep_realized.set(false); - // clear state assembly.regulators.update(|regs| regs.clear()); assembly.elements.update(|elts| elts.clear()); @@ -923,9 +920,6 @@ pub fn TestAssemblyChooser() -> View { "irisawa-hexlet" => load_irisawa_hexlet_assemb(assembly), _ => () }; - - // resume realization - assembly.keep_realized.set(true); }); }); diff --git a/app-proto/src/engine.rs b/app-proto/src/engine.rs index e6ffa25..9f4c688 100644 --- a/app-proto/src/engine.rs +++ b/app-proto/src/engine.rs @@ -50,40 +50,6 @@ pub fn project_point_to_normalized(rep: &mut DVector) { rep.scale_mut(0.5 / rep[3]); } -// given a sphere's representation vector, change the sphere's half-curvature to -// `half-curv` and then restore normalization by contracting the representation -// vector toward the curvature axis -pub fn change_half_curvature(rep: &mut DVector, half_curv: f64) { - // set the sphere's half-curvature to the desired value - rep[3] = half_curv; - - // restore normalization by contracting toward the curvature axis - const SIZE_THRESHOLD: f64 = 1e-9; - let half_q_lt = -2.0 * half_curv * rep[4]; - let half_q_lt_sq = half_q_lt * half_q_lt; - let mut spatial = rep.fixed_rows_mut::<3>(0); - let q_sp = spatial.norm_squared(); - if q_sp < SIZE_THRESHOLD && half_q_lt_sq < SIZE_THRESHOLD { - spatial.copy_from_slice( - &[0.0, 0.0, (1.0 - 2.0 * half_q_lt).sqrt()] - ); - } else { - let scaling = half_q_lt + (q_sp + half_q_lt_sq).sqrt(); - spatial.scale_mut(1.0 / scaling); - rep[4] /= scaling; - } - - /* DEBUG */ - // verify normalization - let rep_for_debug = rep.clone(); - console::log_1(&JsValue::from( - format!( - "Sphere self-product after curvature change: {}", - rep_for_debug.dot(&(&*Q * &rep_for_debug)) - ) - )); -} - // --- partial matrices --- pub struct MatrixEntry { @@ -425,9 +391,20 @@ pub fn realize_gram( // start the descent history let mut history = DescentHistory::new(); + // handle the empty-assembly case + let assembly_dim = guess.ncols(); + if assembly_dim == 0 { + let result = Ok( + ConfigNeighborhood { + config: guess.clone(), + nbhd: ConfigSubspace::zero(0) + } + ); + return Realization { result, history } + } + // find the dimension of the search space let element_dim = guess.nrows(); - let assembly_dim = guess.ncols(); let total_dim = element_dim * assembly_dim; // scale the tolerance From 03d6cf068777e5dbe65f4a13b4dee5087b89ec7b Mon Sep 17 00:00:00 2001 From: Aaron Fenyes Date: Thu, 24 Jul 2025 16:09:26 -0700 Subject: [PATCH 2/6] Flag our workaround for a Sycamore batching bug Add a reminder to remove the workaround once the bug is fixed. --- app-proto/src/components/add_remove.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/app-proto/src/components/add_remove.rs b/app-proto/src/components/add_remove.rs index ef5851c..a685482 100644 --- a/app-proto/src/components/add_remove.rs +++ b/app-proto/src/components/add_remove.rs @@ -18,6 +18,16 @@ pub fn AddRemove() -> View { // this call is batched to avoid redundant realizations. // it updates the element list and the regulator list, // which are both tracked by the realization effect + /* TO DO */ + // it would make more to do the batching inside + // `insert_element_default`, but that will have to wait + // until Sycamore handles nested batches correctly. + // + // https://github.com/sycamore-rs/sycamore/issues/802 + // + // the nested batch issue is relevant here because the + // assembly loaders in the test assembly chooser use + // `insert_element_default` within larger batches state.assembly.insert_element_default::(); }); } From 2bae8d3df9de198deeb148040ad2c39d35c2b7d3 Mon Sep 17 00:00:00 2001 From: Aaron Fenyes Date: Thu, 24 Jul 2025 16:32:18 -0700 Subject: [PATCH 3/6] Prevent unused imports for engine debug output In the process, switch from the `web-sys` crate's `console::log_1` function to Sycamore's more flexible `console_log` macro. The latter works both inside and outside the browser, so we can use it without checking whether we're compiling to WebAssembly. --- app-proto/src/engine.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/app-proto/src/engine.rs b/app-proto/src/engine.rs index 9f4c688..8b5d982 100644 --- a/app-proto/src/engine.rs +++ b/app-proto/src/engine.rs @@ -1,7 +1,10 @@ use lazy_static::lazy_static; use nalgebra::{Const, DMatrix, DVector, DVectorView, Dyn, SymmetricEigen}; use std::fmt::{Display, Error, Formatter}; -use web_sys::{console, wasm_bindgen::JsValue}; /* DEBUG */ + +/* DEBUG */ +#[cfg(not(feature = "dev"))] +use sycamore::prelude::console_log; // --- elements --- @@ -167,10 +170,8 @@ impl ConfigSubspace { /* DEBUG */ // print the eigenvalues - #[cfg(all(target_family = "wasm", target_os = "unknown"))] - console::log_1(&JsValue::from( - format!("Eigenvalues used to find kernel:{}", eig.eigenvalues) - )); + #[cfg(not(feature = "dev"))] + console_log!("Eigenvalues used to find kernel:{}", eig.eigenvalues); // express the basis in the standard coordinates let basis_std = proj_to_std * &basis_proj; From ca57fbce86cb11dc9557655c21ae368601c49976 Mon Sep 17 00:00:00 2001 From: Aaron Fenyes Date: Mon, 28 Jul 2025 10:45:56 -0700 Subject: [PATCH 4/6] Correct the indentation of an empty line --- app-proto/src/engine.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app-proto/src/engine.rs b/app-proto/src/engine.rs index 8b5d982..9d238a3 100644 --- a/app-proto/src/engine.rs +++ b/app-proto/src/engine.rs @@ -403,7 +403,7 @@ pub fn realize_gram( ); return Realization { result, history } } - + // find the dimension of the search space let element_dim = guess.nrows(); let total_dim = element_dim * assembly_dim; From eafb133f8da8b35a297ff5b057f3a9ab04113cdc Mon Sep 17 00:00:00 2001 From: Aaron Fenyes Date: Tue, 29 Jul 2025 00:45:33 -0700 Subject: [PATCH 5/6] Drop eigenvalue logging from `symmetric_kernel` --- app-proto/src/engine.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/app-proto/src/engine.rs b/app-proto/src/engine.rs index 9d238a3..55051fe 100644 --- a/app-proto/src/engine.rs +++ b/app-proto/src/engine.rs @@ -2,10 +2,6 @@ use lazy_static::lazy_static; use nalgebra::{Const, DMatrix, DVector, DVectorView, Dyn, SymmetricEigen}; use std::fmt::{Display, Error, Formatter}; -/* DEBUG */ -#[cfg(not(feature = "dev"))] -use sycamore::prelude::console_log; - // --- elements --- pub fn point(x: f64, y: f64, z: f64) -> DVector { @@ -168,11 +164,6 @@ impl ConfigSubspace { ).collect::>().as_slice() ); - /* DEBUG */ - // print the eigenvalues - #[cfg(not(feature = "dev"))] - console_log!("Eigenvalues used to find kernel:{}", eig.eigenvalues); - // express the basis in the standard coordinates let basis_std = proj_to_std * &basis_proj; From 779c0260bb65d921cc1c162ff67fe21c6c22f3d8 Mon Sep 17 00:00:00 2001 From: Aaron Fenyes Date: Tue, 29 Jul 2025 13:48:40 -0700 Subject: [PATCH 6/6] Explain why the empty-assembly case is special --- app-proto/src/engine.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app-proto/src/engine.rs b/app-proto/src/engine.rs index 55051fe..602fc57 100644 --- a/app-proto/src/engine.rs +++ b/app-proto/src/engine.rs @@ -383,7 +383,9 @@ pub fn realize_gram( // start the descent history let mut history = DescentHistory::new(); - // handle the empty-assembly case + // handle the case where the assembly is empty. our general realization + // routine can't handle this case because it builds the Hessian using + // `DMatrix::from_columns`, which panics when the list of columns is empty let assembly_dim = guess.ncols(); if assembly_dim == 0 { let result = Ok(