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

d3viz: Interactive visualization of Theano compute graphs #3322

Merged
merged 53 commits into from Sep 14, 2015
Merged

d3viz: Interactive visualization of Theano compute graphs #3322

merged 53 commits into from Sep 14, 2015

Conversation

@cangermueller
Copy link
Contributor

@cangermueller cangermueller commented Aug 24, 2015

d3viz extends Theano’s printing module
to interactively visualize compute graphs. Instead of creating a static
picture, it creates an HTML file, which can be opened with any
web-browser. d3viz allows

  • to zoom to different regions and to move graphs via drag and drop,
  • to position nodes both manually and automatically,
  • to retrieve additional information about nodes and edges such as
    their data type or definition in the source code,
  • to edit node labels,
  • to visualizing profiling information, and
  • to explore nested graphs such as OpFromGraph nodes.

This pull request includes the fully documented and tested d3viz module, as well as an user guide with examples.

@abergeron
Copy link
Member

@abergeron abergeron commented Aug 28, 2015

While I agree that this is not likely to cause problems in its current form, using six will help make the code look like the rest of the code and make sure that future changes will not break compatibility.

Also, I think python 2 will stay alive for a couple of years still.

@cangermueller
Copy link
Contributor Author

@cangermueller cangermueller commented Sep 1, 2015

I removed full-fledged external javascript libraries, used six to iterate, updated docstrings, and hopefully fixed all flake errors.

@carriepl
Copy link
Contributor

@carriepl carriepl commented Sep 2, 2015

There are still full versions of the external javascript libraries in the folder doc/library/d3viz/examples/d3viz/js/
Also, Travis seems unhappy but I don't have access to it's results at the moment.

@carriepl
Copy link
Contributor

@carriepl carriepl commented Sep 3, 2015

Finally got access to Travis' results. It complains that, in FAST_COMPILE, the tests for the 3d visualization are failing with a message like "Failed to import pydot. Please install pydot."

@nouiz
Copy link
Member

@nouiz nouiz commented Sep 3, 2015

pydot is an optional dependency. If it is not present, skip the test.

On Thu, Sep 3, 2015 at 10:39 AM, carriepl notifications@github.com wrote:

Finally got access to Travis' results. It complains that, in FAST_COMPILE,
the tests for the 3d visualization are failing with a message like "Failed
to import pydot. Please install pydot."


Reply to this email directly or view it on GitHub
#3322 (comment).

@cangermueller
Copy link
Contributor Author

@cangermueller cangermueller commented Sep 4, 2015

@carriepl, which js library do you mean? There is no minimized version for d3viz.js and d3-context-menu.js, which I implemented myself.

@cangermueller
Copy link
Contributor Author

@cangermueller cangermueller commented Sep 4, 2015

@nouiz, how can I skip the nose test? Alternative, we could add a requirements.txt files with pydot.

@carriepl
Copy link
Contributor

@carriepl carriepl commented Sep 4, 2015

@cangermueller : doc/library/d3viz/examples/d3viz/js/dagre-d3.js and doc/library/d3viz/examples/d3viz/js/graphlib-dot.js

Also, to skip a test :

from theano.tests.unittest_tools import SkipTest
def test_something():
    if my_condition:
        raise SkipTest("This is the reason for skipping the test")
    ...
@cangermueller
Copy link
Contributor Author

@cangermueller cangermueller commented Sep 4, 2015

I removed the full version of dagre-d3.js and graphlib-dot.js from my repository. They should also disappear in the pull request.

Without pydot print, all tests in d3viz/tests fail. Hence, it does not make sense to skip a single tests. One solution would be to skip all d3viz tests if pydot is missing. What do you think?

@carriepl
Copy link
Contributor

@carriepl carriepl commented Sep 4, 2015

This also makes sense. For instance, you can look at the file https://github.com/Theano/Theano/blob/master/theano/sandbox/cuda/tests/test_dnn.py. It raises a single SkipTest exception at the beginning of the file when the necessary libraries are not available.

@carriepl
Copy link
Contributor

@carriepl carriepl commented Sep 4, 2015

Btw, the files you deleted in your repository still show up in your pull-request

@cangermueller
Copy link
Contributor Author

@cangermueller cangermueller commented Sep 7, 2015

@carriepl, I added and removed the full javascript libraries. Now, they don't appear anymore in the latested repository.

@cangermueller
Copy link
Contributor Author

@cangermueller cangermueller commented Sep 8, 2015

Finally, Travis does not complaint anymore!

@carriepl
Copy link
Contributor

@carriepl carriepl commented Sep 8, 2015

@cangermueller Actually, you have a commit adding back the full libraries in theano/d3viz/js/ and then deleting them. But the full libraries in doc/library/d3viz/examples/d3viz/js/ are still there.

@carriepl
Copy link
Contributor

@carriepl carriepl commented Sep 8, 2015

Addendum, these two additional commits that add and remove the same things should be simply removed. And, if possible, the original commits that added the full libraries can be altered (either via amend or interactive rebase) to prevent the full libraries from ever being in the commit history.

@carriepl
Copy link
Contributor

@carriepl carriepl commented Sep 11, 2015

Good. All that remains is the addendum I mentioned to keep the history clean (and therefore the time to do a checkout reasonable).

@cangermueller
Copy link
Contributor Author

@cangermueller cangermueller commented Sep 11, 2015

@carriepl, I removed the add/remove commits from the history. I did not alter the commit that adds the full javascript libraries in the first instance, since many following commits depend on it, and it would be a pain to change all of them. Travis does not complain now any more.

@carriepl
Copy link
Contributor

@carriepl carriepl commented Sep 14, 2015

@cangermueller, Ok I think at this point it's ready for merge. @abergeron, @nouiz and I had a look and what we asked for was done and Travis is also happy with this PR.

Thanks a lot for all of this. It will be very handy to have in Theano.

carriepl added a commit that referenced this pull request Sep 14, 2015
d3viz: Interactive visualization of Theano compute graphs
@carriepl carriepl merged commit aae36ad into Theano:master Sep 14, 2015
2 checks passed
2 checks passed
Scrutinizer 18 updated code elements
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@cangermueller
Copy link
Contributor Author

@cangermueller cangermueller commented Sep 14, 2015

Cool, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants