tdaverse/ggtda

convenience geom layers

Closed this issue · 10 comments

The persistence stat layers are currently paired by default to basic geom layers whose aesthetics are not ideal. We can create some convenience geom layers that simply use better default aesthetics but otherwise pass to the basic geoms. Start with an illustrative geom_disk() to pair with stat_disk().

I'm pretty sure i meant the simplicial complex layers here. An upcoming branch proposes a convenience geom for faces to which stat_disk(), stat_vietoris2(), and stat_cech2() default.

Will submit a PR to master after revising the README to reflect these changes. : )

@rrrlw do we want any other convenience geoms, i.e. for the barcode and persistence layers? I don't remember if the goal here was limited to the point cloud annotation layers or extended to the diagram layers.

rrrlw commented

To be honest, I don't remember either. However, I think that a PR would be appropriate here; maybe if we feel something is missing (or get user feedback about it), we can modify/add stuff in a future update of ggtda?

@rrrlw sure, that sounds reasonable. The README revision is still on my list, and i'll stop with the convenience layers there for now.

@rrrlw heads-up: The convenience branch checks out on its own with the most recent master merged. If it checks out on your end, merge it into master?

rrrlw commented

Running devtools::test() returns 4 warnings and 13 skips for me. I'm guessing the skips are fine, but are the warnings (due to NaN values returned with the stat_cech* tests) a problem?

They should probably be resolved before submission to CRAN. Sorry about that—i noticed them but then moved on. (Note also that the vdiffr tests may cause issues as we work on this branch.)

Commit 5e140fc seems to resolve the problem, which resulted from some arguments to acos() being slightly larger than 1 (or smaller than -1) due to rounding errors. The fix just converts such values back to exactly 1 (-1) before feeding them to acos(), subject to a tolerance that we can change later if necessary. I feel icky about it, but the imprecision is basically irrelevant to the actual image (except in that it preserves faces that would be missing under the original, more strict step). : )

rrrlw commented

Yeah I was having a similar issue in the vignette where a diameter of 1 wasn’t creating the entire 1-skeleton (missing edge between points “exactly” 1 apart). Overall, I think it’s a reasonable fix - thanks!