Regression in stability of curvature while nudging position #86

New issue

Closed

opened 2025-05-19 08:20:27 +00:00 by Vectornaut · 2 comments

Vectornaut commented

2025-05-19 08:20:27 +00:00

Member

At pull request #80 (commit 360ce12), when you nudge the position of a sphere, its curvature stays pretty stable, like we want. At the next pull request, #82 (commit a2478fe), we lose this desired behavior: nudging the position of a small sphere makes its curvature drift rapidly.

Reproduction

In the general test assembly, select Deimos—the small yellow sphere.
- Deimos has half-curvature 2, and the first component of its representation vector is 0.
Hold D to nudge Deimos in the x direction until the first component of its representation vector reaches 4.
- At pull request #80, the curvature of Deimos stays between 1.99 and 2.
- At pull request #82, the curvature of Deimos drifts to over 2.6.

At pull request #80 (commit 360ce12), when you nudge the position of a sphere, its curvature stays pretty stable, like we want. At the next pull request, #82 (commit a2478fe), we lose this desired behavior: nudging the position of a small sphere makes its curvature drift rapidly. ### Reproduction 1. In the general test assembly, select Deimos—the small yellow sphere. - *Deimos has half-curvature 2, and the first component of its representation vector is 0.* 2. Hold **D** to nudge Deimos in the $x$ direction until the first component of its representation vector reaches 4. - *At pull request #80, the curvature of Deimos stays between 1.99 and 2.* - *At pull request #82, the curvature of Deimos drifts to over 2.6.*

Vectornaut added the

bug

label 2025-05-19 08:20:27 +00:00

Vectornaut changed title from ~~Regression: curvature of small spheres drifts rapidly while nudging~~ to Regression in stability of curvature during position nudges

2025-05-19 08:27:36 +00:00

Vectornaut changed title from ~~Regression in stability of curvature during position nudges~~ to Regression in stability of curvature while nudging position

2025-05-19 08:28:38 +00:00

Vectornaut commented

2025-05-20 01:06:05 +00:00

Author

Member

Bug found! It's a simple problem that should have a simple fix, but I'll describe it thoroughly because I think it's worth recalling how nudging currently works.

The context

The nudging routine does these steps:

Move the assembly along a tangent vector of the solution variety (typically moving it off the solution variety).
Normalize each element, if necessary, to get the assembly back onto the normalized configuration variety.
Realize the assembly to get it back onto the solution variety.

The normalization step helps preserve the curvature of a sphere while nudging its position.

The problem

Right now, only spheres need to be normalized, because moving along a tangent vector doesn't affect the normalization of points. Pull request #82 therefore encloses the normalization in a conditional that's supposed to test whether the element is a sphere:

if elt.type_id() == TypeId::of::<Sphere>() { /* normalize */ }

However, the test always evaluates to false, so the normalization never runs!

A solution

Checking the type of a trait object isn't very idiomatic, so I think we should give the Element trait a method like

normalize_mut_rep(&self, rep: &mut DVector<f64>)

that does nothing to rep in its implementation for Point and runs the desired normalization routine on rep in its implementation for Sphere. The nudging routine could then use that method in the normalization step.

Bug found! It's a simple problem that should have a simple fix, but I'll describe it thoroughly because I think it's worth recalling how nudging currently works. #### The context The nudging routine does these steps: 1. Move the assembly along a tangent vector of the solution variety (typically moving it off the solution variety). 2. Normalize each element, if necessary, to get the assembly back onto the normalized configuration variety. 3. Realize the assembly to get it back onto the solution variety. The normalization step helps preserve the curvature of a sphere while nudging its position. #### The problem Right now, only spheres need to be normalized, because moving along a tangent vector doesn't affect the normalization of points. Pull request #82 therefore encloses the normalization in a [conditional](https://code.studioinfinity.org/StudioInfinity/dyna3/src/commit/a2478febc128b218d833bc3e904482f2055865cb/app-proto/src/assembly.rs#L715) that's supposed to test whether the element is a sphere: ```rust if elt.type_id() == TypeId::of::<Sphere>() { /* normalize */ } ``` However, the test always evaluates to `false`, so the normalization never runs! #### A solution Checking the type of a trait object isn't very idiomatic, so I think we should give the `Element` trait a method like ```rust normalize_mut_rep(&self, rep: &mut DVector<f64>) ``` that does nothing to `rep` in its implementation for `Point` and runs the desired [normalization routine](https://code.studioinfinity.org/StudioInfinity/dyna3/src/commit/a2478febc128b218d833bc3e904482f2055865cb/app-proto/src/assembly.rs#L716-L722) on `rep` in its implementation for `Sphere`. The nudging routine could then use that method in the normalization step.

Vectornaut referenced this issue

2025-05-21 01:23:31 +00:00

Dispatch normalization routines correctly #87