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

Simplify the realization triggering system #105

Merged
glen merged 6 commits from Vectornaut/dyna3:reactive-realization-cleanup into main 2025-07-31 22:21:34 +00:00
Conversation 2 Commits 6 Files changed 4 +1 -1
1 changed files with 1 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit ca57fbce86 - Show all commits

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

@ -403,7 +403,7 @@ pub fn realize_gram(
);
return Realization { result, history }
}
glen marked this conversation as resolved Outdated
glen commented 2025-07-26 15:43:33 +00:00
Outdated
Copy link

(a) Why do we now need an empty-assembly special case, when we didn't before? Or is it just for efficiency?

(b) Could this be expanded relatively easily to the more common no-set-regulators case?

(a) Why do we now need an empty-assembly special case, when we didn't before? Or is it just for efficiency? (b) Could this be expanded relatively easily to the more common no-set-regulators case?
Vectornaut commented 2025-07-28 17:57:18 +00:00
Outdated
Copy link

(a) Why do we now need an empty-assembly special case, when we didn't before? Or is it just for efficiency?

The realization triggering system on the main branch never tries to realize an empty assembly, so we've never had to worry about the empty-assembly case.

As I mentioned in the pull request description, the realization triggering system on the incoming branch allows some unnecessary realizations in exchange for simplicity. One of those unnecessary realizations happens when we construct an empty assembly using the constructor Assembly::new.

As it turns out, the realization function on the main branch panics when you try to realize an empty assembly. This is because an empty assembly has an empty (zero by zero) Hessian, and the DMatrix::from_columns function, which we use to build the Hessian, can't handle an empty list of columns.

(b) Could this be expanded relatively easily to the more common no-set-regulators case?

Sure. We could give PartialMatrix an is_empty method and then expand the early return condition to:

assembly_dim == 0 || (gram.is_empty() && frozen.is_empty())
> (a) Why do we now need an empty-assembly special case, when we didn't before? Or is it just for efficiency? The realization triggering system on the main branch never tries to realize an empty assembly, so we've never had to worry about the empty-assembly case. As I mentioned in the pull request description, the realization triggering system on the incoming branch allows some unnecessary realizations in exchange for simplicity. One of those unnecessary realizations happens when we construct an empty assembly using the constructor `Assembly::new`. As it turns out, the realization function on the main branch panics when you try to realize an empty assembly. This is because an empty assembly has an empty (zero by zero) Hessian, and the `DMatrix::from_columns` function, which we use to build the Hessian, can't handle an empty list of columns. > (b) Could this be expanded relatively easily to the more common no-set-regulators case? Sure. We could give `PartialMatrix` an `is_empty` method and then expand the early return condition to: ```rust assembly_dim == 0 || (gram.is_empty() && frozen.is_empty()) ```
glen commented 2025-07-28 19:21:03 +00:00
Outdated
Copy link

Any reason not to go ahead and do that in this PR? Seems like more bang for not many more bucks -- as long as we are checking for one kind of trivial case, why not exclude many more trivial cases? Seems to me like there's no downside...

Any reason not to go ahead and do that in this PR? Seems like more bang for not many more bucks -- as long as we are checking for one kind of trivial case, why not exclude many more trivial cases? Seems to me like there's no downside...
Vectornaut commented 2025-07-28 21:55:26 +00:00
Outdated
Copy link

Any reason not to go ahead and do that in this PR?

On second thought, we'd actually have to use a separate conditional for the unconstrained case, because the tangent space would be different. When there are no elements, we return the zero subspace as the tangent space; when there are elements but no constraints, we want to return the whole configuration vector space as the tangent space.

I'd recommend leaving realize_gram mostly as it is. When there are no constraints, realize_gram still exits before doing a Cholesky decomposition, which is the most costly part of the optimization step: the loop breaks halfway through the first iteration, in the block with the comment "stop if the loss is tolerably low." After the break, we use ConfigSubspace::symmetric_kernel to find the tangent space. This costs more computationally than hard-coding the tangent space for the unconstrained case, but I think it makes maintenance easier.

Note that to get the spectrum for the diagnostics panel, realize_gram finds the eigenvalues of the Hessian on every step. I'm not factoring this into the cost considerations above, because I'd only want to do this in development builds.

> Any reason not to go ahead and do that in this PR? On second thought, we'd actually have to use a separate conditional for the unconstrained case, because the tangent space would be different. When there are no elements, we return the zero subspace as the tangent space; when there are elements but no constraints, we want to return the whole configuration vector space as the tangent space. I'd recommend leaving `realize_gram` mostly as it is. When there are no constraints, `realize_gram` still exits before doing a Cholesky decomposition, which is the most costly part of the optimization step: the loop breaks halfway through the first iteration, in the block with the comment "stop if the loss is tolerably low." After the break, we use `ConfigSubspace::symmetric_kernel` to find the tangent space. This costs more computationally than hard-coding the tangent space for the unconstrained case, but I think it makes maintenance easier. Note that to get the spectrum for the diagnostics panel, `realize_gram` finds the eigenvalues of the Hessian on every step. I'm not factoring this into the cost considerations above, because I'd only want to do this in development builds.
glen commented 2025-07-28 23:13:46 +00:00
Outdated
Copy link

@Vectornaut wrote in #105 (comment):

On second thought, we'd actually have to use a separate conditional for the unconstrained case, because the tangent space would be different. When there are no elements, we return the zero subspace as the tangent space; when there are elements but no constraints, we want to return the whole configuration vector space as the tangent space.

How do those differ? When there are no elements, the whole configuration vector space is the zero space...

@Vectornaut wrote in https://code.studioinfinity.org/StudioInfinity/dyna3/pulls/105#issuecomment-3114: > On second thought, we'd actually have to use a separate conditional for the unconstrained case, because the tangent space would be different. When there are no elements, we return the zero subspace as the tangent space; when there are elements but no constraints, we want to return the whole configuration vector space as the tangent space. How do those differ? When there are no elements, the whole configuration vector space is the zero space...
Vectornaut commented 2025-07-29 17:19:59 +00:00
Outdated
Copy link

How do those differ? When there are no elements, the whole configuration vector space is the zero space...

Good point: in principle, a ConfigSubspace variant representing a full subspace could be used for both empty assemblies and unconstrained assemblies. I've prototyped a full subspace representation and saved the code locally in case we want it later. In the process, however, I realized that we'll never actually encounter an unconstrained assembly, because every element comes with a normalization constraint. I'm now strongly against adding special handling for unconstrained assemblies, because during normal operation, this code will never run.

> How do those differ? When there are no elements, the whole configuration vector space is the zero space... Good point: in principle, a `ConfigSubspace` variant representing a full subspace could be used for both empty assemblies and unconstrained assemblies. I've prototyped a full subspace representation and saved the code locally in case we want it later. In the process, however, I realized that we'll never actually encounter an unconstrained assembly, because every element comes with a normalization constraint. I'm now strongly against adding special handling for unconstrained assemblies, because during normal operation, this code will never run.
glen commented 2025-07-29 19:36:57 +00:00
Outdated
Copy link

OK, yes, I had forgotten that our coordinates are themselves underdetermined and that we use essentially the same mechanism to keep the coordinates for a given item in the form we want as to place an actual Euclidean-geometric constraint. So I agree, there's no point in any code for unconstrained assemblies other than the totally empty assembly. So yes go ahead and leave the special case for empty assemblies; please just add a comment as to why it must be handled specially. Thanks!

OK, yes, I had forgotten that our coordinates are themselves underdetermined and that we use essentially the same mechanism to keep the coordinates for a given item in the form we want as to place an actual Euclidean-geometric constraint. So I agree, there's no point in any code for unconstrained assemblies other than the totally empty assembly. So yes go ahead and leave the special case for empty assemblies; please just add a comment as to why it must be handled specially. Thanks!
Vectornaut commented 2025-07-29 20:52:35 +00:00
Outdated
Copy link

So yes go ahead and leave the special case for empty assemblies; please just add a comment as to why it must be handled specially. Thanks!

Done in commit 779c026!

> So yes go ahead and leave the special case for empty assemblies; please just add a comment as to why it must be handled specially. Thanks! Done in commit 779c026!
// find the dimension of the search space
let element_dim = guess.nrows();
let total_dim = element_dim * assembly_dim;