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

Katz centrality #370

Merged
merged 15 commits into from
May 31, 2023
Merged

Katz centrality #370

merged 15 commits into from
May 31, 2023

Conversation

acombretrenouard
Copy link
Contributor

New Katz centrality algorithms, computed with a power-series over the adjacency matrix.
Another option would be a direct matrix inversion (computationally harder ?).

The usual contribution pipeline runs without errors (pytest, isort ., etc...).

acombretrenouard and others added 5 commits May 20, 2023 21:18
New function in , computes the Katz-centrality for hypergraph.
This commit does not passe all tests yet (bug with the namespace when importing ).
The function katz_centrality was not mentionned in __all__ in file xgi/algorithms/centrality.py.
Docstring was also completed in katz_centrality.
Run pylint xgi/ --disable all --enable W0611, isort . and black ..
Updated CHANGELOG.md.
@codecov
Copy link

codecov bot commented May 20, 2023

Codecov Report

Patch coverage: 95.23% and project coverage change: +0.15 🎉

Comparison is base (5cff847) 90.74% compared to head (1d4c046) 90.90%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #370      +/-   ##
==========================================
+ Coverage   90.74%   90.90%   +0.15%     
==========================================
  Files          42       42              
  Lines        3113     3178      +65     
==========================================
+ Hits         2825     2889      +64     
- Misses        288      289       +1     
Impacted Files Coverage Δ
xgi/algorithms/centrality.py 97.39% <95.00%> (-0.53%) ⬇️
xgi/algorithms/shortest_path.py 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Collaborator

@leotrs leotrs left a comment

Choose a reason for hiding this comment

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

Hey @acombretrenouard thank you so much for your work on this! This is almost ready, just left some minor things :)

tests/algorithms/test_centrality.py Outdated Show resolved Hide resolved
tests/algorithms/test_centrality.py Show resolved Hide resolved
xgi/algorithms/shortest_path.py Show resolved Hide resolved
xgi/algorithms/centrality.py Outdated Show resolved Hide resolved
xgi/algorithms/centrality.py Outdated Show resolved Hide resolved
If index is set to True, nodedict will contain the nodes ids, keyed by their indice in vector c.
<c[key]> will be the centrality of node <nodedict[key]>.

Note
Copy link
Collaborator

Choose a reason for hiding this comment

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

Check out this link to see how to markup LaTeX within a docstring.

Also, the section title should be Notes, note Note (sphinx is very fussy about this...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put variables in between ``, is that ok ?

CHANGELOG.md Outdated Show resolved Hide resolved
@maximelucas
Copy link
Collaborator

Thanks a lot Antoine, great work.
I left a few additional comments mainly to make sure we give enough details in the docs.

Just a question: what values can this take?
I remember we had to 1-normalise other centralities to avoid problems (like negative values), and maybe it would be better to make this one consistent with the others in that respect. @nwlandry I think you did this?

@nwlandry
Copy link
Collaborator

Thanks a lot Antoine, great work. I left a few additional comments mainly to make sure we give enough details in the docs.

Just a question: what values can this take? I remember we had to 1-normalise other centralities to avoid problems (like negative values), and maybe it would be better to make this one consistent with the others in that respect. @nwlandry I think you did this?

Yeah, I did this in in the clique eigenvector centrality because the eigenvector is not guaranteed to be in the correct direction (it can have all positive entries or all negative entries).

@acombretrenouard
Copy link
Contributor Author

Thank you for your comments ! I tried to take them into account. The current version should be much better.

@acombretrenouard
Copy link
Contributor Author

Hello again !

I added the minor fixes you mentioned.

On the normalization : the Katz-centrality counts the number of walks starting from a single node. This is a local feature (as there is the exponential attenuation of walks). In this sense, the Katz centrality is a sort of 'clustering coefficient'.
For the moment the code does not normalize the vector.

Conversely, normalizing would add the global contribution of every other node. Maybe this is what we want to reflect ?
To me, the problem with that would be that a node with a given neighborhood would have different centrality values depending on the more distant parts of the graph...

Copy link
Collaborator

@nwlandry nwlandry left a comment

Choose a reason for hiding this comment

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

Nice work! The only thing is that there is a duplicate import but I will fix that once it's merged in.

@nwlandry nwlandry merged commit 792ec0b into xgi-org:main May 31, 2023
@maximelucas
Copy link
Collaborator

Thanks Antoine !

@acombretrenouard acombretrenouard deleted the katz-centrality branch June 1, 2023 07:12
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.

4 participants