Implement Persistence Landscapes #48
Conversation
…visual" This reverts commit 2ad1396.
Add test for duplicate bars.
Make pers_landscape_approx compute on instantiation. Fix typo in plot_landscape_approx
Remove __future__ import annotations, remove type hinting from arithmetic methods
Add two edge cases in plapprox
|
Sweet! I’ll get ready to merge and deploy everything this afternoon. Anything in particular you’d like me to call out when I make the Twitter announcement? |
|
I think we're all set. I will add tests and work on tweaking the results of the ML notebook within the next month or so with a new PR. And I can't think of anything in particular for the announcement. Thanks for all your help with this! |
|
ahh actually I would prefer if we went ahead and added the tests before we release the code. Ive found testing often uncovers bugs in the implementation, and once it's out, it's easy to never go back and add the tests. I'm okay letting this sit here a while till the tests are in place, Thank you! |
|
I totally get that. I will try to move it up on my list of things to do, but it will still probably be a couple weeks before everything is done. |
|
@sauln I'm getting back to this, but I can't import the package. >>> import persim
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "E:\Miniconda\envs\baseenv\lib\site-packages\persim\__init__.py", line 8, in <module>
from .landscapes import *
ModuleNotFoundError: No module named 'persim.landscapes' I hope to have the additional tests added soon and I was hoping you could take care of this in the meantime. |
|
@catanzaromj I got this error too and managed to fix it. I can push my fixes to the calder branch of your fork. |
Removed .landscapes import to avoid functions being in two different places (persim.PersLandscapeApprox() and persim.landscapes.PersLandscapeApprox()). Now, default usage would be persim.landscapes.PersLandscapeApprox()
|
Shoot, sorry about this! Thanks for taking care of it @calderds |
Landscapes install
The former approximate p_norm method used vector norm from numpy, giving incorrect results. Refactor exact p_norm and move to auxiliary as _p_norm. Now both exact and approximate p_norm methods call _p_norm
Move `death_vector` into auxiliary, remove `linear_combination` since it is no longer needed, rename `snap_PL` to `snap_pl`
Add tests for the methods of PersLandscapeExact, PersLandscapeApprox, auxiliary functions, and PersistenceLandscaper
|
I finished adding a bunch of tests and found a bug in the |
|
@sauln I think we're ready to go. Anything else that needs to be taken care of? |
| @@ -1,2 +1,3 @@ | |||
| include *.txt | |||
| include *.md | |||
| include persim/landscapes/* | |||
There was a problem hiding this comment.
This is surprising. Is this necessary? I thought setuptools will automatically include any *.py files.
|
@sauln Done. Thanks for all your help with this. |
|
Rad, let's ship it! |
|
Thank you @catanzaromj , @gabbyangeloro , and @calderds ! I'm very glad to have an implementation of landscapes in the library finally. |
This PR adds persistence landscapes with visualization and basic arithmetic methods. We implement two PL classes, one approximate for faster computations and one exact with no approximations. The approximate class is defined on a grid and we tried to use numpy vectorization when possible to make it fast, especially for things like linear combinations. There is a quicker 2D plotting method and a slower 3D variant as well. We also implemented L_p norms and transformer methods for the two classes.