Infinite recursion calling loadmesh with a non AbstractMeshKey

[quffaro]

dset = DeltaSet1D()
add_vertices!(dset, 1)
dset |> loadmesh

This code will produce an error

Stacktrace:
     [1] loadmesh(s::DeltaSet1D, subdivision::Circumcenter)
       @ CombinatorialSpaces.Meshes ~/.julia/packages/CombinatorialSpaces/Wg7wx/src/Meshes.jl:74--- the last 1 lines are repeated 79982 more times ---

This occurs because loadmesh will call subdivide_duals!(loadmesh(s), subdivision), and iterate.

I imagine there should be some way to eyeball whether this will recurse. I'm willing to assign this to myself.

[lukem12345]

Hi, which version of loadmesh is this calling? Can you please do @which? This function is not meant to be used this way.

[quffaro]

It's calling it here:

loadmesh(s)
     @ CombinatorialSpaces.Meshes ~/.julia/packages/CombinatorialSpaces/Wg7wx/src/Meshes.jl:74

[lukem12345]

Ok. It seems that there are two issues here. The typing on loadmesh should not allow this usage.

[quffaro]

Is it possible to distinguish at the type level "bad" objects from "good" ones so we could just overload loadmesh, or should there be code which checks for recursive badness (and throws an error?)?

[jpfairbanks]

It is this fallback method here.

This should have a generic type like AbstractMeshIdentifier that has concrete subtypes for all our existing meshes so that the fallback method isn't defined for Any. Then we could do

loadmesh(m::AbstractDeltaSet) = m so that loadmesh is idempotent.

[lukem12345]

Yes this should subtype AbstractMeshKey. The name "load" was originally chosen because we were literally loading a mesh from a zip file (i.e. Artifacts). What Matt wants is a helper function that takes a primal mesh, creates a dual mesh, and subdivides it.

[lukem12345]

If we add the functionality that you suggested James, we should not keep calling this function loadmesh, because this falls into classic object-oriented naming scheme badness. Besides it being now misleadingly tied to a particular way that an object lives in memory.

It would be more Julian methinks to just overload EmbeddedDeltaDualComplex2D at this point.

[lukem12345]

But currently EmbeddedDeltaDualComplex2D already does this for a given EmbeddedDeltaSet2D. It just doesn't subdivide it in the same step. We could just do that, subdividing either Circumcentrically, Barycentrically, or None. And pick either circum. or bary. as default.

[lukem12345]

But currently EmbeddedDeltaDualComplex2D already does this for a given EmbeddedDeltaSet2D. It just doesn't subdivide it in the same step. We could just do that, subdividing either Circumcentrically, Barycentrically, or None. And pick either circum. or bary. as default.

[lukem12345]

@quffaro Please add minimal reproducible code that demonstrates this behavior by calling EmbeddedDeltaDualComplex1D and subdivide_duals!, not using the loadmesh function.

[quffaro]

Affirmative!

Point3D = GeometryBasics.Point3{Float64}
primal=EmbeddedDeltaSet1D{Bool, Point3D}()
add_vertices!(primal, 5, point=[Point3D(i, i^2, 0) for i in -2:2])
add_edges!(primal, 1:4, 2:5, edge_orientation=true)

ds=EmbeddedDeltaDualComplex1D{Bool, Float64, Point3D}(primal)
subdivide_duals!(ds, Barycenter()) 

dss = copy(ds)
subdivide_duals!(dss, Barycenter())

dss==ds