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

Improvements to forceatlas2 algorithm #488

Merged
merged 7 commits into from
Oct 14, 2017
Merged

Improvements to forceatlas2 algorithm #488

merged 7 commits into from
Oct 14, 2017

Conversation

jbcrail
Copy link
Contributor

@jbcrail jbcrail commented Oct 5, 2017

The datatype of the coordinates is now consistent. We also check for edges pointing to themselves.

Related to #485

@jbcrail jbcrail requested a review from jbednar October 5, 2017 18:15
nlen = len(nodes)
if 'id' in nodes:
index = dict(zip(nodes['id'].values, range(nlen)))
else:
index = dict(zip(nodes.index.values, range(nlen)))

if 'weight' not in edges:
if params.ignore_weights or 'weight' not in edges:
edges = edges.copy()
edges['weight'] = np.ones(len(edges))
Copy link
Member

Choose a reason for hiding this comment

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

This seems like a very expensive step, and doesn't seem feasible as an approach in general, but I guess we can merge and come back to that later.

jbednar and others added 3 commits October 5, 2017 18:36
Instead of copying the dataframe and creating another single-value
column if the weights are ignored, we use an if-statement to specify the
default weight.
Creating a temporary list of tuples that are immediately thrown away is
redundant and wasteful.
@jbcrail
Copy link
Contributor Author

jbcrail commented Oct 13, 2017

After profiling the memory usage for sparse matrix creation, it surprisingly turned out that removing the temporary tuple list had a far larger impact than the updated steps taken when ignoring weights. The overall time didn't change noticeably in either case.

@jbednar
Copy link
Member

jbednar commented Oct 13, 2017

Ok, sounds great! I'll merge when tests pass.

@jbednar jbednar merged commit 99a65ed into master Oct 14, 2017
@jbednar jbednar deleted the issue-485-more branch October 14, 2017 13:00
jbednar pushed a commit that referenced this pull request Oct 30, 2017
* Replace constants with parameters

* Check for loops

* Optionally ignore weights during layout
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.

2 participants