Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add representative-cycles #26

Open
daytenjs opened this issue May 6, 2022 · 8 comments
Open

Add representative-cycles #26

daytenjs opened this issue May 6, 2022 · 8 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@daytenjs
Copy link

daytenjs commented May 6, 2022

Found on c++ package branch here: https://github.com/Ripser/ripser/tree/representative-cycles

I'm trying to do it but its not my skillset

@corybrunson
Copy link
Member

On one hand, i am hesitant to incorporate experimental tools from the source into a release of the port.

On the other hand, if the experimental tool is mature, then i would be glad to work in a branch on a version that would enable R users to access it.

@rrrlw glad to follow your lead on this.

@daytenjs
Copy link
Author

daytenjs commented May 6, 2022

The Representative Cocycles branch is implemented in the Python implementation of Ripser if thats any help regarding the maturity. The representative-cocycles would also serve my purposes, but the representative-cycles is what I want. Having both of course would be even more useful :)

@rrrlw
Copy link
Member

rrrlw commented May 10, 2022

@corybrunson It would be great if you could work on this in a branch!

I think it would be most appropriate to have the current features packaged/released as the next major update. Then, we can include representative cycles/cocycles in a major/minor update after. Being in a branch would still allow users to use the functionality by installing the development version in the branch via the remotes package; but the CRAN release would remain stable throughout (and only be updated once the cycles/cocycles functionality has withstood the test of time). How does this sound?

@corybrunson
Copy link
Member

@rrrlw i can work a bit on this over the summer—please assign me and check in if you remember to in June and July. : )

@rrrlw rrrlw added the enhancement New feature or request label May 11, 2022
@corybrunson corybrunson removed their assignment Jul 9, 2022
@corybrunson
Copy link
Member

@daytenjs i'm sorry to walk back my plans! This is doable, but i would need more time than i currently have. Though i now agree that both features (cocycles and cycles) are high-priority and ready to write.

@corybrunson corybrunson added the help wanted Extra attention is needed label Jul 9, 2022
@daytenjs
Copy link
Author

daytenjs commented Jul 9, 2022

I'd be happy to try and help, I know little C++ (I do want to expand on that) but have much experience with R, having even written a few unreleased packages. I would love the chance to contribute, let me know!

@corybrunson
Copy link
Member

corybrunson commented Jul 9, 2022

@daytenjs if you'd like to test the waters in a new branch & fork then i will be glad to support you along the way! From my own similar experience, it can take a while to get comfortable with C++, but building something is the best way to.

Here're my observations from the source code:

  • The representative-cocycles and representative-cycles branches of Ripser seem to have no inherent conflicts.
  • The (co)cycles are printed by Ripser, and this code was amended to store the cocycles in SciKit-TDA. We want to do something similar through Rcpp.
  • The naming conventions vary greatly between Ripser and SciKit-TDA; this can cause confusion. I urge us to adhere to those of Ripser by default, as the current C++ code does.

@michiexile
Copy link

representative-cocycles would be a highly valuable addition to Ripser in my opinion. It is crucial for doing circular coordinates (for eg intrinsic phase angles for quasi-periodic timeseries)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants