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

Implementation of linearized_directed constructor #70

Merged
merged 8 commits into from
Feb 27, 2023

Conversation

d-schindler
Copy link
Collaborator

This PR is to provide linearized MS for directed networks as discussed in #69

@d-schindler d-schindler added the enhancement New feature or request label Feb 24, 2023
@d-schindler d-schindler requested a review from arnaudon February 24, 2023 09:35
@d-schindler
Copy link
Collaborator Author

We need to set $\alpha=1$ (no teleportation) as a default for large graphs because otherwise the quality functions is a dense matrix, which leads to memory errors.

@d-schindler
Copy link
Collaborator Author

We should have constructors_kwargs as an argument in run() such that one can change teleportation conveniently.

@d-schindler
Copy link
Collaborator Author

I think we need to improve the code for constructors by using more scipy.sparse commands.

@d-schindler
Copy link
Collaborator Author

@arnaudon , I think this is ready to merge after your improvements. could you e.g. provide a kwarg in run to set the teleportation parameter alpha?

@codecov-commenter
Copy link

Codecov Report

Merging #70 (3554cfe) into master (33890b7) will not change coverage.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff            @@
##            master       #70   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         7           
  Lines          552       576   +24     
=========================================
+ Hits           552       576   +24     
Flag Coverage Δ
pytest 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/pygenstability/constructors.py 100.00% <100.00%> (ø)
src/pygenstability/pygenstability.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@arnaudon
Copy link
Collaborator

Here you are!

@arnaudon
Copy link
Collaborator

Let me know if it's ok, and we can merge!

@d-schindler
Copy link
Collaborator Author

Do you keep the print statements on purpose? Also the print("alpha1")?

src/pygenstability/constructors.py Outdated Show resolved Hide resolved
src/pygenstability/constructors.py Outdated Show resolved Hide resolved
@arnaudon
Copy link
Collaborator

Oups, I just removed them!

@arnaudon
Copy link
Collaborator

is it good to merge like that @d-schindler ?

@d-schindler
Copy link
Collaborator Author

Yes, I think it's fine now!

@d-schindler
Copy link
Collaborator Author

Before you merge, could you delete this from benchmarking.py, it's deprecated: with_spectral_decomp=True

@arnaudon arnaudon merged commit bb25818 into master Feb 27, 2023
@arnaudon arnaudon deleted the linearized_directed branch February 27, 2023 14:12
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.

3 participants