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

included functions from publication #400

Merged
merged 8 commits into from
Jul 25, 2023
Merged

included functions from publication #400

merged 8 commits into from
Jul 25, 2023

Conversation

maximelucas
Copy link
Collaborator

I added two functions I made here: https://github.com/maximelucas/HOI_shape_sync_differently/blob/main/utils.py

  • shuffle_hyperedges(H, order, p): shuffle hyperedges with a given probability
  • node_swap(H, nid1, nid2, order): swap node membership in edges of given order
  • added tests for both

I put them in a new file generators/randomizing.py.

@codecov
Copy link

codecov bot commented Jun 13, 2023

Codecov Report

Patch coverage: 92.06% and project coverage change: +0.01 🎉

Comparison is base (d7e59cc) 90.95% compared to head (d4dbaf0) 90.97%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #400      +/-   ##
==========================================
+ Coverage   90.95%   90.97%   +0.01%     
==========================================
  Files          47       48       +1     
  Lines        3882     3944      +62     
==========================================
+ Hits         3531     3588      +57     
- Misses        351      356       +5     
Impacted Files Coverage Δ
xgi/generators/randomizing.py 91.80% <91.80%> (ø)
xgi/generators/__init__.py 100.00% <100.00%> (ø)

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

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.

Overall looks good! My biggest question would be if you could mirror some of the same syntax in the double_edge_swap method of the hypergraph class. No worries if it doesn't make sense.

@maximelucas
Copy link
Collaborator Author

Good point, let me have a look. You mean like the docstrings and variable names?

Btw, should we also move double_edge_swap to randomizing.py? Or should we at least have them in the same file? Since they're similar in ideas.

@nwlandry
Copy link
Collaborator

I see double edge swap as a fundamental edge operation so I would keep in in Hypergraph for now. I didn't look very closely, but the contents of node_swap() seem to have similarities to what is implemented in double_edge_swap(). Either node_swap() could call double_edge_swap() or just use some of the same algorithm/variable names. But maybe there's not as direct a correspondence as I thought.

@maximelucas
Copy link
Collaborator Author

Finally had a better look.

  • functionality: they are similar but different. double_edge_swap() picks two nodes and two edges, and swaps the membership of those two nodes in those two edges. node_swap() also picks two nodes, but then swaps their membership in all edges that they are part of, not just two.

  • naming: It's your function, but looping over all the edges that contain them. Because of that, it felt more natural for me to call it "node swap" rather than edge swap. Your function is more "symmetric" wrt to nodes and edges, I guess it could also be called "double_node_swap"?

  • location: to me in might make more sense conceptually to put both of them in randomizing.py? I understand that you see the swap as a fundamental operation. hypergraph.py now contains almost only methods to remove/add nodes/edges/attributes, "creating structure". In my mind, the two swap functions take an existing structure, and modify it by switching things around. In my case, I wrote node_swap() with randomization in mind, to degree sequences intact but changing degree correlations between orders. Maybe yours is less random as you pick the edges to use.

  • implementation: node_swap() is clearly based on your implementation of double_edge_swap() (variable names, structure, etc.). I added a few initial checks to make sure that the queried nodes exist etc. Conceptually it could be implemented as a loop where each iteration uses your function in practice I remember I thought that would be more of a hassle than anything.

Happy to change anything, let me know what you think.

@nwlandry
Copy link
Collaborator

@maximelucas: I'm good with you merging it, thanks for the additional clarification. We can probably revisit whether double_edge_swap should be in the Hypergraph class.

@maximelucas maximelucas merged commit 584a495 into main Jul 25, 2023
19 checks passed
@maximelucas maximelucas deleted the from-natcom branch July 25, 2023 14:38
tlarock pushed a commit to tlarock/xgi that referenced this pull request Aug 3, 2023
* feat: added shuffle_hyperedges + tests

* feat: added shuffle_hyperedges + tests

* fix: filenames

* fix: tests

* style: black and isort

* feat: added function node_swap + test

* style: black and isort

* fix: error from python 3.11
maximelucas added a commit that referenced this pull request Aug 4, 2023
* Added weights option to to_line_graph function.

* Added tests for weighted line graph implementation.

* Added s=2 test.

* Added encapsulation dag code.

* Added encapsulation dag tests.

* Removed unnecessary combinations import.

* Updated docstring.

* included functions from publication (#400)

* feat: added shuffle_hyperedges + tests

* feat: added shuffle_hyperedges + tests

* fix: filenames

* fix: tests

* style: black and isort

* feat: added function node_swap + test

* style: black and isort

* fix: error from python 3.11

* attempt at listing the available statistics (#405)

* attempt at listing the available statistics

* fixing previous errors

* Attempt at implementing suggestions by nich

* second attempt at displaying the titles

* another trial

* titles in bold

* update of the panel for dihypergraph stats

* minor fixes

* change the hierarchical structure

* minor change

* moving MultiDi-stats in the right place

* adding decorators to the admonition box

* moving up the decorators

* up-version

* feat: enforce equal aspect for circular layout #430 (#432)

* feat: enforce equal aspect for circular layout #430

* changed implementation of set_aspect

* Refactor draw module (#435)

* refact: draw module

* style: black and isort

* fix #331 (#438)

* Added the ability for users to access the optional arguments of NetworkX layout functions. (#439)

* Added another test.

* Refactored relations to subset_types. Exposed and documented empirical relations filtering function. Documentation improvements.

* Update xgi/convert/encapsulation_dag.py documentation

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

* Fix typo in xgi/convert/encapsulation_dag.py

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

* Updates to documentation.

* Minor refactor.

---------

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>
Co-authored-by: Thomas Robiglio <83019028+thomasrobiglio@users.noreply.github.com>
Co-authored-by: Nicholas Landry <nicholas.landry.91@gmail.com>
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.

None yet

2 participants