A Circle defined with mixed units can parametrize incorrectly

[mikeingold]

MWE

This circle function produces a Circle on the xy-plane, centered on the origin, and with a specified radius. Internally, the other Point and Vecs used to construct the Circle are unitless, which Meshes promotes as implicitly representing distances in meters.

using Meshes
using Unitful

function circle(radius)
	origin = Meshes.Point(0, 0, 0)
	x = Meshes.Vec(1, 0, 0)
	y = Meshes.Vec(0, 1, 0)
	plane = Meshes.Plane(origin, x, y)
	Meshes.Circle(plane, radius)
end

ts = range(0, 1, length=5)

If the circle is defined with a 1 mm radius in a meters Quantity type, then the result is as expected.

julia> circle(0.001u"m").(ts)
5-element Vector{Point{𝔼{3}, CoordRefSystems.Cartesian3D{CoordRefSystems.NoDatum, Quantity{Float64, 𝐋, Unitful.FreeUnits{(m,), 𝐋, nothing}}}}}:
 Point(x: 0.001 m, y: 0.0 m, z: 0.0 m)
 Point(x: 0.0 m, y: 0.001 m, z: 0.0 m)
 Point(x: -0.001 m, y: 0.0 m, z: 0.0 m)
 Point(x: 0.0 m, y: -0.001 m, z: 0.0 m)
 Point(x: 0.001 m, y: 0.0 m, z: 0.0 m)

If the circle is defined with a 1 mm radius in a millimeters Quantity type, then the resulting coordinates correspond instead to a circle with a 1 meter radius.

julia> circle(1.0u"mm").(ts)
5-element Vector{Point{𝔼{3}, CoordRefSystems.Cartesian3D{CoordRefSystems.NoDatum, Quantity{Float64, 𝐋, Unitful.FreeUnits{(m,), 𝐋, nothing}}}}}:
 Point(x: 1.0 m, y: 0.0 m, z: 0.0 m)
 Point(x: 0.0 m, y: 1.0 m, z: 0.0 m)
 Point(x: -1.0 m, y: 0.0 m, z: 0.0 m)
 Point(x: 0.0 m, y: -1.0 m, z: 0.0 m)
 Point(x: 1.0 m, y: 0.0 m, z: 0.0 m)

Debugging

I haven't had time yet to debug this but I suspect it's caused by the single-argument ustrips here:

Meshes.jl/src/geometries/primitives/disk.jl

Lines 36 to 46 in eeedffa

	function (d::Disk)(ρ, φ)
	if (ρ < 0 || ρ > 1) || (φ < 0 || φ > 1)
	throw(DomainError((ρ, φ), "d(ρ, φ) is not defined for ρ, φ outside [0, 1]²."))
	end
	T = numtype(lentype(d))
	l = T(ρ) * radius(d)
	sφ, cφ = sincospi(2 * T(φ))
	u = ustrip(l * cφ)
	v = ustrip(l * sφ)
	plane(d)(u, v)
	end

[juliohm]

Thank you @mikeingold for reporting. I believe you nailed the root of the issue. Didn't test, but it looks like we should the two-arg method of ustrip here.