networkx / nx-guides

Examples and Jupyter Notebooks about NetworkX
https://networkx.org/nx-guides/
Creative Commons Zero v1.0 Universal
185 stars 105 forks source link

Exploratory facebook notebook #14 issue #38

Closed stef4k closed 2 years ago

stef4k commented 3 years ago

I have created a facebook circles notebook as described in #14 . The analysis consists of the graph representation, basic topological attributes, centrality measures, clustering effects, bridges, assortativity and communities. However, I am not sure about:

rossbar commented 3 years ago

Thanks for the contribution @stef4k ! I've only had time to skim it so far, but this seems like a great addition!

I have used many graph representations in different sections to showcase the findings, which take time to load due to the plethora of nodes and edges

This is indeed something we'll have to iterate on - there is currently a timeout of 30 seconds/cell, which is why the ci is failing. This limit is intentional - the idea being that for a tutorial to be effective as an interactive document, the computation times should be manageable. If it takes several minutes to run the analysis in a single cell, that's unlikely to be as useful to users who are trying to run the notebooks interactively. I haven't investigated in detail yet, so maybe there are opportunities to speed some things up. I think the most effective strategy will likely be to limit long-running analyses to a subset of data to illustrate/test things out (this is a best-practice when it comes to exploratory data analysis), but we will see.

I will aim to start taking a closer look at this ASAP!

stef4k commented 3 years ago

Thank you for the quick response @rossbar . From what I understand, the main problem is the nx.draw_networkx() command due to the large network size. Some different options I thought are:

Lastly, the other commands with long waiting time are nx.diameter, nx.average_shortest_path_length, nx.shortest_path, nx.centrality.betweenness_centrality and nx.centrality.closeness_centrality

stef4k commented 2 years ago

@rossbar is there anything I can or should do here?

rossbar commented 2 years ago

Thanks for the ping @stef4k , I let this slip through the cracks "/

I'll aim to take another look at it early next week!

MridulS commented 2 years ago

Great work @stef4k! We should get this in ASAP. But as @rossbar mentions that this notebook covers a lot so it may take a lot of back and forth. We could merge this in and then make changes as required, and we could also do a "pair-programming-review" over Zoom/Hangouts to reduce the overhead of the back and forth review if @stef4k and @rossbar are okay with it?

stef4k commented 2 years ago

Thank you for the kind words. @rossbar and @MridulS I am okay with any plan you suggest to follow

stef4k commented 2 years ago

@rossbar thank you for the suggestions. I accepted them and fetched the upstream changes both into my main and notebook-development branch of my origin. Also, I invited you as a collaborator in my origin to make things easier. Do you want me to merge the notebook-development branch into the main in my origin?

rossbar commented 2 years ago

Do you want me to merge the notebook-development branch into the main in my origin?

Nope that shouldn't be necessary. I will open PRs to stef4k/notebook-development instead, which you should see on your fork. I recently opened stef4k/nx-guides#2 with a proposal to reuse the plot layouts to cut the notebook runtime by nearly a factor of 2.

I wanted to open a PR instead of pushing directly to your branch because these are relatively big changes, and I wanted to give you a chance to review them and question/comment rather than just forcing stuff directly into your work! Once you've had a chance to review the PR on your fork, you can merge it (if you're happy with it) and the changes should automatically show up here without the need for any more git acrobatics :)

stef4k commented 2 years ago

@rossbar I totally understand. This way will let me learn and understand more things, while I am also participating.

However, after committing the changes I saw that there was a new error at build ubuntu and in detail at install dependencies, as you can see in the screenshot:

image

rossbar commented 2 years ago

Ah, thanks for the heads up - time to update the Python version for the CI checks!

rossbar commented 2 years ago

Ok @stef4k , the CI is fixed now on the main branch which I merged into this branch and pushed up so everything should be running again. Just make sure to git pull the latest changes.

Also it looks like the performance improvements + the more relaxed runtime restrictions on main combined so that the circleCI execution no longer times out :tada: This means the preview feature should now be working: https://119-40210422-gh.circle-artifacts.com/0/site/_build/html/content/exploratory_notebooks/facebook_notebook.html.