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

added better visu #69

Closed
wants to merge 2 commits into from
Closed

Conversation

MathieuCarriere
Copy link
Collaborator

Added a few cells to improve visualization in cover complex notebook (better scaling in 3D scatter plot + interactive visu of cover complex)

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@@ -1062,7 +1062,7 @@
{
Copy link
Member

@mglisse mglisse Oct 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vincent did something in #68 to hide these messages until GUDHI/gudhi-devel#882 is fixed.


Reply via ReviewNB

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I am proposing is to add at the beginning of the notebook:

# Add some verbosity to cover complexes methods
verbose=False

Then everywhere it is used, like:

cover_complex = GraphInducedComplex(
    input_type='point cloud', cover='voronoi', min_points_per_node=0,
    graph="rips", rips_threshold=None, N=100, beta=0., C=10,
    voronoi_samples=100, verbose=verbose) # Notice the 'verbose=verbose' - could be named differently

Like that, we can deactivate the verbosity (by default) on the notebook, and reactivate it in debug phase.
Like that we won't have the too long lines like:

10%
...
100%
...
Computing geodesic distances to seed 0...
...
Computing geodesic distances to seed 99...
...

@mglisse
Copy link
Member

mglisse commented Oct 19, 2023

Some of the visualizations won't be visible on github, but that sounds ok.
It would be nice if at some point in the future the keplermapper script became a Python function 😉
@VincentRouvreau is the CI error that the notebook takes too long to render?

@VincentRouvreau
Copy link
Contributor

@VincentRouvreau is the CI error that the notebook takes too long to render?

I tried locally, but I have another error.
I have matplotlib 3.5.2 that explicitly raise NotImplementedError ("Axes3D currently only supports the aspect argument ") when ax.set_aspect('equal') if ax is a 3d projection.

@MathieuCarriere
Copy link
Collaborator Author

OK I tried something else for 3D visu with equal axes, hopefully it works now

@VincentRouvreau
Copy link
Contributor

OK I tried something else for 3D visu with equal axes, hopefully it works now

Yes, it now works fine for me. It seems the error is a timeout error, cf. this stckoverflow. Not to be fixed on this PR, I will try something on another PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants