mesa icon indicating copy to clipboard operation
mesa copied to clipboard

Added directed edges functionality to D3 Visualization

Open AdamCordner641 opened this issue 4 years ago • 12 comments

Added DirectedGraph example (VirusOnNetworkDirected). Appended arrowhead SVG to D3.js module. Fixes issue https://github.com/projectmesa/mesa/issues/667 (partially). Based off pull request https://github.com/projectmesa/mesa/pull/846.

AdamCordner641 avatar Mar 30 '22 14:03 AdamCordner641

In the .git/config of your Mesa git repo, make sure to add

[pull]
    rebase = true

This will prevent the noisy merge commits when you pull from Mesa main to update your branch.

rht avatar Mar 31 '22 02:03 rht

@AdamCordner641 My impression is you are new to open source development so let us know if you have any questions. Everyday I am learning something new.

tpike3 avatar Mar 31 '22 10:03 tpike3

Hi all. I have pushed changes which I believe should resolve the issues. Let me know if anything else needs changed :)

Also, here is an example of an undirected graph and a directed graph generated with these changes: undirected endlarge

AdamCordner641 avatar Mar 31 '22 20:03 AdamCordner641

I diff-ed the model.py of this PR with the undirected version:

18c18
<     return sum(1 for a in model.grid.get_all_cell_contents() if a.state is state)
---
>     return sum([1 for a in model.grid.get_all_cell_contents() if a.state is state])
33c33
< class VirusOnNetwork(Model):
---
> class VirusOnNetworkDirected(Model):
  1. return sum(1 for a in model.grid.get_all_cell_contents() if a.state is state) is preferred
  2. this means that the model graph is actually undirected, because there is no difference. erdos_renyi_graph has directed as an argument, which is not used

rht avatar Apr 01 '22 06:04 rht

Here is a diff of server.py:

8c8
< from .model import VirusOnNetwork, State, number_infected
---
> from .model import VirusOnNetworkDirected, State, number_infected
37c37,39
<             "tooltip": f"id: {agents[0].unique_id}<br>state: {agents[0].state.name}",
---
>             "tooltip": "id: {}<br>state: {}".format(
>                 agents[0].unique_id, agents[0].state.name
>             ),
47a50,51
>             "directed": True,
>             "marker_size": "large"
68c72
<         ratio_text = "&infin;" if ratio is math.inf else f"{ratio:.2f}"
---
>         ratio_text = "&infin;" if ratio is math.inf else "{0:.2f}".format(ratio)
138c142
<     VirusOnNetwork, [network, MyTextElement(), chart], "Virus Model", model_params
---
>     VirusOnNetworkDirected, [network, MyTextElement(), chart], "Directed Virus Model", model_params
140c144
< server.port = 8521
---
> server.port = 8521

The "...".format(...) needs to be replaced with the more modern f-strings.

rht avatar Apr 01 '22 06:04 rht

@AdamCordner641 I'm not sure how it happened, but you have accidentally included a commit in the main branch in this PR. Have you made sure to add https://github.com/projectmesa/mesa/pull/1223#issuecomment-1084010129 ? And then you do git pull upstream main, assuming projectmesa/mesa remote is called upstream in your .git/config.

rht avatar Apr 01 '22 06:04 rht

Codecov Report

Merging #1223 (8c908bc) into main (2841d98) will decrease coverage by 0.11%. The diff coverage is n/a.

:exclamation: Current head 8c908bc differs from pull request most recent head 537ea88. Consider uploading reports for the commit 537ea88 to get more accurate results

@@            Coverage Diff             @@
##             main    #1223      +/-   ##
==========================================
- Coverage   89.05%   88.93%   -0.12%     
==========================================
  Files          19       19              
  Lines        1306     1310       +4     
  Branches      260      260              
==========================================
+ Hits         1163     1165       +2     
- Misses        104      106       +2     
  Partials       39       39              
Impacted Files Coverage Δ
mesa/space.py 93.97% <0.00%> (-0.49%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2841d98...537ea88. Read the comment docs.

codecov[bot] avatar Apr 02 '22 11:04 codecov[bot]

Pushed and tested fixes which should resolve the issues, but let me know if I missed anything :)

AdamCordner641 avatar Apr 04 '22 21:04 AdamCordner641

I still think that the example in this PR doesn't showcase directed networks. It's behaviorally identical to the undirected version. To make it different, I think we need to implement get_neighbors in a new class called DirectedNetworkSpace [1] in such a way that it returns either of:

  • all in and out neighbors
  • all in neighbors only
  • all out neighbors only

[1] let's call it "space" instead of "grid"

rht avatar Apr 05 '22 11:04 rht

I think we should defer this PR, and do a separate PR that implements DirectedNetworkSpace with tests. Once this is done, we can revisit this PR.

rht avatar Apr 05 '22 11:04 rht

I think we should defer this PR, and do a separate PR that implements DirectedNetworkSpace with tests. Once this is done, we can revisit this PR.

I concur. @AdamCordner641 Thank you very much for all the hard work you have put into this. Your work definitely revealed some work we need to do. I hope you can use the code you developed in your project and would be happy for any assistance you could provide on building out a DirectedNetworkSpace.

tpike3 avatar Apr 05 '22 12:04 tpike3

I think we should defer this PR, and do a separate PR that implements DirectedNetworkSpace with tests. Once this is done, we can revisit this PR.

I concur. @AdamCordner641 Thank you very much for all the hard work you have put into this. Your work definitely revealed some work we need to do. I hope you can use the code you developed in your project and would be happy for any assistance you could provide on building out a DirectedNetworkSpace.

No problem! Thank you both for helping with PR feedback. I can definitely use these changes I have made in my project. I would also like to be able to use the get_neighbors functionality rht described in my project, so I'll try to make time to have a go at implementing a DirectedNetworkSpace class.

AdamCordner641 avatar Apr 05 '22 12:04 AdamCordner641

Thank you for the PR and sorry for never putting it forward

If this is still actively followed please re-open the PR at https://github.com/projectmesa/mesa-viz-tornado But note that we are moving towards a new pure python frontend

Corvince avatar Aug 31 '23 20:08 Corvince