Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

spatial/r3: Transformer interface and scope of r3 package #1809

Open
soypat opened this issue May 26, 2022 · 8 comments
Open

spatial/r3: Transformer interface and scope of r3 package #1809

soypat opened this issue May 26, 2022 · 8 comments

Comments

@soypat
Copy link
Contributor

soypat commented May 26, 2022

Is it too late to consider the following change to r3? Feel like a huge opportunity was missed when naming the Rotate method for Rotation. This would really simplify things on my end for the sdf package.

// Could be defined on user side, no need for it to be in r3
type Transformer interface {
    Apply(Vec) Vec
}

var _ Transformer = Rotation{}
var _ Transformer = Transform{}

Suggest adding Apply method and marking Rotate as deprecated as was done with Vec methods.

@kortschak
Copy link
Member

When the package was designed it was not intended to be something bigger than storing spatial information for statistical methods and rendering static spatial data; this explains the reason that it is the way that it is. It looks like it is growing significantly in scope with the new additions. If it is going to grow into a generalised set of spatial data packages (I'm not opposed to this, but it brings additional complexity and already seems to be happening) then how it looks should be discussed.

On the plus side of the discussion, it adds the possibility of easily supporting dualquat/dualcomplex transformations to r2 and r3. I think the name of the interface needs work.

@kortschak
Copy link
Member

On names, I'd suggest Transform => Affine and then make the interface type Transformer interface { Transform(Vec) Vec }.

To start the discussion, it would be helpful if you could outline where it is that you want to take the package(s) (if r3 moves, then r2 needs to as well to maintain API congruence) and what are the motivations for the extension. This can happen at gonum-dev if you want to flesh things out.

@soypat
Copy link
Contributor Author

soypat commented May 28, 2022

Transform => Affine

Yes! I had been thinking just that the other day. I'll apply this change to the Transform PR one of these days.

As for the end goal/future additions I do have a few comments:

I envision gonum/r3 as being a solid but basic foundation to build 3D data manipulation programs.

About the shape types

  1. Triangle type is a much needed for computational geometry. It is the bread and butter of surface representation
  2. Tetrahedron type brings in the basic r3 volumetric representation. Yes, Box does volumes but is very limited and cannot represent arbitrary volumes. I'd even argue that r3.Tetrahedron, not r3.Triangle, is the r3 complement to r2.Triangle.

This may seem like a slippery slope: we add Tetrahedron, who's stopping us from adding Dodecahedron tomorrow? I don't see the benefit from having a repository of shape types, I do see the benefit in having the most basic shape for representing volume. So today I'd say Tetrahedron is the first and last volumetric shape that would be a welcome addition to gonum. Same goes for "flat" or surface shapes. Triangle is enough.

Transform/Affine

I feel Affine was waiting to happen. Gonum already had some form of linear transformations of vecs with Rotation, Transform just brought the generalization for all transformations.

More spatial primitives

I'd really like to have an exported Line type (and if it's not too much to ask, a Plane type too!). I spent a lot of time researching minimum distance calculations and whatnot- from my point of view these types would save the user loads of time. These would also open the floor for interesting examples (under spatial/r3) I'd like to add to gonum: like calculating the minimum distance to a triangle (very interesting math behind this).

@kortschak
Copy link
Member

I envision gonum/r3 as being a solid but basic foundation to build 3D data manipulation programs.

Do you have a concrete project in mind?

This may seem like a slippery slope: we add Tetrahedron, who's stopping us from adding Dodecahedron tomorrow?

No, I agree, the 2-simplex and 3-simplex satisfy the needs of r2 and r3 (r3 requiring both if r3.Tetrahedron is added). This is the end. Box exists only because of coordinate convenience.

Line may be justified, but Plane is just r3.Triangle.

Note that there are implications with graph here (see @vladimir-ch's work here that handles data needed to represent meshes).

@soypat
Copy link
Contributor Author

soypat commented May 28, 2022

Do you have a concrete project in mind?

I have actually been working with these types on a playground styled project for testing out ideas https://github.com/soypat/go-play3d. So far it has helped me create the stl to sdf import function and start work on a tetrahedral sdf mesher. I have created the tetrahedrons using a similar datastructure to dcel. I use the Tetrahedron type to validate aspect ratios and plot.

Line may be justified, but Plane is just r3.Triangle.

I agree plane data is already contained in Triangle, though there are memory and performance costs to using the Triangle type instead of a specialized Plane type. See plane. The plane type is best constructed with a normal unit vector to aid with distance calculations. If using triangle type then Sqrt could be called up to two times per distance calculation (if using r3.Unit).

@soypat
Copy link
Contributor Author

soypat commented May 28, 2022

These are a few more functions that I'd like to have in gonum which I forgot to mention. These are commonly used in 3D algorithms (sorry about the informality of these proposals).

// Hessian returns the hessian matrix of a scalar field defined by f
// about a point p. tol defines the discretization step used to approximate
// the hessian.
func Hessian(p Vec, tol float64, f func(Vec) float64) *Mat // Method on *Mat?

// https://en.wikipedia.org/wiki/Gradient
func Gradient(p Vec, tol float64, f func(Vec) float64) Vec

// Jacobian returns the jacobian matrix of a vector field...
// have had no real use for this one yet but is here for completeness.
func Jacobian(p Vec, tol float64, f func(Vec) Vec) *Mat  // Method on *Mat?

Edit: Divergence was missing here

// https://en.wikipedia.org/wiki/Divergence
func Divergence(p Vec, tol float64, f func(Vec) Vec) float64

@kortschak
Copy link
Member

I think this needs discussion; gonum-dev would be better that here. Hopefully we can get some others to contribute to thinking about this because it's a large set of changes that significantly changes the focus of the package. After that a formal proposal with a plan of the API explcitly described would be good to prevent scope creep.

I'm not against this (next to graph things, spatial things are a thing that I enjoy playing with, but this is partly why I'm reluctant to just say yes here because I'm concerned about my own biases).

@soypat soypat changed the title spatial/r3: missed opportunity on a Transformer interface? spatial/r3: Transformer interface and scope of r3 package May 28, 2022
@soypat
Copy link
Contributor Author

soypat commented May 31, 2022

More complete interface:

type Transformer interface {
    Apply(Vec) Vec
    // Returns the inverse transform of Transformer
    Inv() Transformer
}

This was referenced Jul 21, 2022
This was referenced Jul 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants