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

Fix eigen decomposition; new feature to store all optimization tries; and updates to documentation #95

Merged
merged 12 commits into from
Nov 14, 2024

Conversation

d-schindler
Copy link
Collaborator

I made a few changes to the code:

  • Fix: Replace linalg.eig with linalg.eigh for the spectral decomposition used for computation of exponentials. We only apply spectral decompostion in the case of undirected graphs where linalg.eigh is the faster method (and we actually found an example where linalg.eig does produce incorrect results for an undirected graph).
  • Feature: I introduced a parameter with_all_tries in the pgs.run() function that stores the information from all optimization tries in the results dictionary under an additional key "all_tries". For some applications it is interesting to look at the different optimizations produced for the same scale, not only at the best one that is selected.
  • Fix: There are a few things I corrected/updated in the documentation.

@d-schindler d-schindler added the enhancement New feature or request label Nov 8, 2024
@d-schindler d-schindler requested a review from arnaudon November 8, 2024 09:53
@d-schindler d-schindler self-assigned this Nov 8, 2024
@d-schindler
Copy link
Collaborator Author

I also renamed block_detection_curve in the optimal scale selection to block_nvi to make it consistent with our figure labels and the terminology in our software paper.

@d-schindler
Copy link
Collaborator Author

I added a figsize parameter to the plot_scan function, which is something I found quite useful.

@d-schindler
Copy link
Collaborator Author

@arnaudon , before we merge this PR we should probably update the online documentation of the module.

@d-schindler
Copy link
Collaborator Author

Further improvement of the documentation: I added the missing division by in the formula for MS with the continuous combinatorial Laplacian.

@d-schindler
Copy link
Collaborator Author

I've also added more references to the formulas for constructors. @arnaudon , I used reference [1] several times and wasn't sure if I should repeat it in every docstring or not.

"""Plot results of pygenstability with matplotlib."""
scales = _get_scales(all_results, scale_axis=scale_axis)
plt.figure(figsize=figsize)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I didn't put this here, so one can call figure before this function, and have more freedom

@arnaudon arnaudon merged commit 6614d19 into master Nov 14, 2024
2 checks passed
@arnaudon arnaudon deleted the improvements branch November 14, 2024 14:52
@arnaudon arnaudon restored the improvements branch November 14, 2024 14:52
@arnaudon arnaudon deleted the improvements branch November 14, 2024 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants