corybrunson / plt

R interface & extension to an adapted Persistence Landscapes Toolbox
https://corybrunson.github.io/plt
GNU General Public License v3.0
0 stars 2 forks source link

enable pl_new to interpret an input matrix (without using as_persistence) #6

Open jamesgolabek22 opened 1 month ago

jamesgolabek22 commented 1 month ago

This issue is similar to issue #3, opened by @a1danbryant .

I reinstalled {plt} from the new branch, so that pl_new() could be utilized. My understanding was as_persistence() would not need to be explicitly called since 1) the naming conflict was resolved, and 2) as_persistence() is called in pl_new().

Below is an example of the difference of the persistence landscape plots for when we use as_persistence() and when we do not.

# Load necessary libraries
library(reprex)
library(plt)
library(TDA)
library(tdaunif)

# correct plot of persistence landscape; uses as_persistence()
set.seed(120246L)
X <- sample_lemniscate_gerono(60, sd = 0.05)
pd <- alphaComplexDiag(X, maxdimension = 1)$diagram
pd[, c(2, 3)] <- sqrt(pd[, c(2, 3)])
pd <- as_persistence(pd)
pl1d <- pl_new(pd, degree = 1, xby = 0.025)
n_levs <- max(pl_num_levels(pl1d))
plot(pl1d, main = "Persistence Landscape", n_levels = n_levs, asp = 1)

# incorrect plot of persistence landscape; doesnt use as as_persistence()
set.seed(120246L)
X <- sample_lemniscate_gerono(60, sd = 0.05)
pd <- alphaComplexDiag(X, maxdimension = 1)$diagram
pd[, c(2, 3)] <- sqrt(pd[, c(2, 3)])
pl1d <- pl_new(pd, degree = 1, xby = 0.025)
n_levs <- max(pl_num_levels(pl1d))
plot(pl1d, main = "Persistence Landscape", n_levels = n_levs, asp = 1)

corybrunson commented 1 month ago

@jamesgolabek22 thank you! I understand the issue. The change from landscape() to pl_new() did not affect the internals of the function, so the same limitations on its use still apply.

This is something that should be addressed before submission to CRAN, so i will leave this issue open for the time being. Specifically, if a user provides a numeric matrix to pl_new(), the function should be able to determine whether it contains only birth/death values (in which case degree should indicate the dimensions of the features) or dimension values as well (in which case degree should specify the dimension to use). Hopefully this won't have counterintuitive aftereffects; it seems natural enough behavior, and would require the input matrix to have either 2 or 3 columns, which should limit ambiguity.

jamesgolabek22 commented 1 month ago

Got it, understood. Thank you!

corybrunson commented 2 weeks ago

@jamesgolabek22 please re-install from the new branch and see if the problem is solved.

In the case of a 2-column matrix, the fix does not expect degree to be passed, since it doesn't need to subset the data. In future, it may be important to tag the PL with the degree of the features from which it was constructed, at which time the warning that degree is present when unexpected is replaced with a warning that degree is missing even when not required.