-
-
Notifications
You must be signed in to change notification settings - Fork 403
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
Kneser Graph Generator #2233
base: master
Are you sure you want to change the base?
Kneser Graph Generator #2233
Conversation
|
Hi. You're right, my bad. I forgot to change the name while refactoring. |
b19aca2
to
afa7b2e
Compare
Codecov Report
@@ Coverage Diff @@
## master #2233 +/- ##
=======================================
Coverage 83.54% 83.54%
=======================================
Files 376 376
Lines 61586 61586
=======================================
Hits 51450 51450
Misses 10136 10136 Continue to review full report in Codecov by Sentry.
|
Just a note that it's strongly preferred to use non-recursive implementations. Recursive functions are prone to stack overflow. This has happened in the past. |
afa7b2e
to
1e8277e
Compare
Hi. I thought I would just give an update here. I have made Edit: Both functions are now iterative. |
1. Fix function name in function body 2. Make igraph_error_t the return type of igraph_i_generate_subsets 3. Use IGRAPH_CHECK and IGRAPH_SUCCESS 4. Remove global variable 5. Style check and cleanup
Use 'igraph_vector_resize()' and remove 'igraph_vector_init()'
9679476
to
6f94af2
Compare
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions. |
What's the status of this PR? @valdaarhun are you waiting for us before you continue? |
Hi. Yes, I was waiting for feedback. I apologize for not sending a reminder, I got caught up with other work. I would like to resume working on this now. |
igraph_integer_t subsets, igraph_vector_int_list_t *vertices) { | ||
|
||
igraph_vector_int_t *temp = igraph_vector_list_get_ptr(vertices, 0); | ||
IGRAPH_CHECK(igraph_vector_resize(temp, k)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since temp
is an igraph_vector_int_t
, you need to use the functions that take an igraph_vector_int_t
. For instance, igraph_vector_int_resize()
instead of igraph_vector_resize()
, igraph_vector_int_update()
instead of igraph_vector_update()
and so on. I haven't checked the correctness of the algorithm yet, this just simply struck my eye.
I haven't checked the correctness of this function, this just simply struck my eye.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I must have missed this. I'll fix it.
No apologies necessary 🙂 If you add your file to src/CMakeLists.txt you can use CMake to build your code and see the warnings. If you add the actual generator function (or maybe for now just something that tests your subset generator) you can also add some tests in If you have any further questions (like about using CMake) don't hesitate to ask. |
Oh, that's nice. I'll definitely do this.
Sure thing. Thanks. |
New feature: Generation of kneser graphs.
Resolves #1876.
So far, I have only implemented functions to calculate$\binom{n}{k}$ and generate k-size subsets. I have made both functions static and put them in
kneser.c
. Please let me know if I should integrate them intoiterators.c
instead.I think it'll be best to finalize the design and implementation of the above functions before implementing the actual graph generator.