Meshes.jl icon indicating copy to clipboard operation
Meshes.jl copied to clipboard

Accept tuples of parametric coordinates besides splatting

Open mikeingold opened this issue 10 months ago • 2 comments

Background

Currently, Geometrys can have parametric functions defined that take as input parametric coordinates in $[0,1]$ and return a Point on the geometry at that coordinate. Since different geometries have different numbers of parametric dimensions (1D lines, 2D areas, 3D volumes, etc), each of these functions can take a different number of input arguments (coordinates).

Additionally, the implicit API here has been that these functions return a Point in Cartesian coordinates. With the ongoing CRS work and impending refactor around this, users may also be interested in these functions returning different formats.

Proposal

We could standardize on a common method signature for parametric functions that uses multiple dispatch to efficiently determine the output type. This could look something like:

# Parametric function for some Geometry with return type Cartesian specified
function (g::Geometry{Dim,T})(t::AbstractVector{T}, ::Type{Cartesian}) where {T<:AbstractFloat}
    ...
    return Cartesian(...)
end

# Parametric function for some Geometry with return type Spherical specified
function (g::Geometry{Dim,T})(t::AbstractVector{T}, ::Type{Spherical}) where {T<:AbstractFloat}
    ...
    return Spherical(...)
end

# Method signature with only coordinates: default to Cartesian
(g::Geometry{Dim,T})(t::AbstractVector{T}) where {T<:AbstractFloat} = g(t, Cartesian)
# Before
g(u, v)

# After
g([u, v])   # implicitly returns a Cartesian coordinate
g([u, v], Cartesian)   # explicitly returns a Cartesian coordinate
g([u, v], Spherical)   # explicitly returns a Spherical coordinate

One benefit of providing the parametric coordinate(s) as a vector-like object is that it might make it easy to write fast generics. For example, when trying to chain this function with multi-dimensional integration code it would be nice to have generic start and stop points like dims = paramdim(g); start = zeros(dims); stop = ones(dim).

mikeingold avatar Mar 27 '24 21:03 mikeingold

Now that I’ve had more time to think about this, I’m not sure the ability to request non-Cartesian outputs makes a lot of sense. The syntax might be confusing since it might lead users to think that they’re specifying parametric coordinates in that system. Also, I suspect that almost all parametric methods will naturally be expressed in Cartesian outputs for their generic cases, so the extra output types will inevitably just shift the conversion from the user side to the internals.

I do still, however, think it’s worth debating whether parametric function arguments should be provided as a vector-like, separate args, or both. Are there any existing benchmarks to characterize these functions performance? That would be a nice starting pointing.

mikeingold avatar Apr 08 '24 18:04 mikeingold

Agree @mikeingold , I believe your second suggestion is worth exploring, i.e., parametrizations with static arrays or tuples instead of relying on the splat operator. Our current syntax is g(u,v,w) for any geometry g, but perhaps we should accept g(uvw) as well.

Will rename the issue accordingly.

juliohm avatar May 01 '24 13:05 juliohm

@mikeingold the new CRS system is in place 🥂 , it would be a good time to review the idea of passing (u,v) coords as a tuple/vec, besides splatting.

juliohm avatar May 22 '24 20:05 juliohm

I haven’t had time lately to keep up with the CRS progress, but now that it’s live I’ll try to check it over the new few days.

mikeingold avatar May 23 '24 02:05 mikeingold

@mikeingold can you please remind what is the motivation for accepting tuples besides the splat? We noticed that supporting both methods leads to uglier code with necessary type annotations to avoid ambiguity, and that splatting shouldn't cause any performance issue when the length of the uv coords is static, which is the case.

juliohm avatar Jun 06 '24 20:06 juliohm

Closing as not needed. Feel free to reopen if necessary.

juliohm avatar Jun 11 '24 14:06 juliohm