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 get_edges() and linalg.incidence_matrix_by_order() #18

Merged
merged 2 commits into from
Sep 19, 2023

Conversation

ab1nash
Copy link
Contributor

@ab1nash ab1nash commented Sep 18, 2023

  1. Exposed incidence_matrix_by_order() to be called directly from an object (eg. hg1.incidence_matrix_by_order(2) is now possible.)
  2. get_edges() had a bug where, while initializing a subhypergraph from a weighted hypergraph the weights were not being provided correctly. This commit fixes said issue.
  3. Fix minor parameter bug in the function linalg.incidence_matrix_by_order()

Please check and let me know if any edge cases are breaking. That can also help when writing tests for this module, which is my eventual plan. Thanks!

linalg.incidence_matrix_by_order()
@ab1nash
Copy link
Contributor Author

ab1nash commented Sep 19, 2023

Hi, I have exposed linalg.incidence_matrix_by_order() without considering the roadmap (Issue #2). So it is okay if you want me to roll that change back (Point 1 above)

@FraLotito
Copy link
Contributor

Hi, thank you for your contribution! As of now, I'm still deciding what is the correct way to handle lin alg methods. As of now I prefer them to not be exposed. Can you roll point 1 back?

Thank you!:)

@ab1nash
Copy link
Contributor Author

ab1nash commented Sep 19, 2023

Hi, no problem at all, I enjoy working on this repo. I have rolled back point 1.

@FraLotito FraLotito merged commit 8c0b8ed into HGX-Team:master Sep 19, 2023
@FraLotito
Copy link
Contributor

Thanks, I'm merging the PR! I'm happy that you're enjoying the repo! :)

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