Add HistogramPlot and minor fix to Plot #569

fvisin · 2015-03-31T19:33:57Z

Add HistogramPlot, an extension to plot histograms of the mean of each component of
a channel
Call push only once at the beginning in Plot, as afterwards it is sufficient to modify the data source for the plot to update

dwf · 2015-03-31T19:35:27Z

blocks/extensions/plot.py

+            If you want the server to accept connections from any address use
+            <ip>= 0.0.0.0.
+
+        Parameter


Parameters*

fvisin · 2015-03-31T19:47:35Z

Wow, @dwf I never received a review so fast! I am working on it.

dwf · 2015-03-31T20:06:44Z

blocks/extensions/plot.py

@@ -4,14 +4,12 @@
 from subprocess import Popen, PIPE

 try:
-    from bokeh.plotting import (curdoc, cursession, figure, output_server,


were curdoc and cursession never used, I guess?

curdoc was used by __setstate__, while cursession() is now unused because I use self.session. This prevents problems when you have multiple extensions plotting (i.e. multiple sessions open)

fvisin · 2015-03-31T20:30:24Z

Travis error doesn't seem related to my PR and I should have addressed all your requests.

dwf · 2015-03-31T20:38:22Z

I restarted Travis but these did in fact look related... the demos run with the plotting extension, and there was an error regarding Bokeh schema.

bartvm · 2015-03-31T20:40:25Z

blocks/extensions/plot.py

@@ -63,9 +61,7 @@ class Plot(SimpleExtension):
    server_url : str, optional
        Url of the bokeh-server. Ex: when starting the bokeh-server with
        ``bokeh-server --ip 0.0.0.0`` at ``alice``, server_url should be
-        ``http://alice:5006``. When not specified the default configured
-        by ``bokeh_server`` in ``.blocksrc`` will be used. Defaults to
-        ``http://localhost:5006/``.


Why did you remove this configuration setting?

Indeed, this documentation looks useful.

Apparently I failed to merge 9c8ad24 for some reason. It's not intentional.

fvisin · 2015-03-31T23:59:57Z

I pushed another minor commit. The changes are mostly aesthetic, but they improve the plot quite a bit in my opinion. If you don't have any other comment, I don't plan to modify it further.

- Add HistogramPlot to plot histograms of the mean of each component of a channel - Call push only once at the beginning in Plot, afterwards it is sufficient to modify the data source for the plot to update

- Visualize the right value on x axis in Plot - Show the axes's labels - Iterate only on the provided keys

fvisin · 2015-04-14T20:08:06Z

Travis build passed, is it ready for merge or I am not noticing something?

fvisin · 2015-04-29T12:52:58Z

Hey, is there a reason why this PR has not been merged yet? If there is something missing can you please point that out?

bartvm · 2015-04-29T15:03:05Z

Personally I'd like to wait till we have blocks-contrib/blocks-extra because it's a lot of code that I wouldn't consider essential to teh core. I'm sorry this has taken so long, we had exams last week and I've simply been too busy to get that sorted out. If people feel there's a need to merge this sooner I'm happy to hear it.

fvisin · 2015-04-30T12:22:07Z

Don't worry! I know you are busy, I just wanted to make sure it wasn't due to something I was missing.

For what concerns the merge, I would like to see it merged so that I can use it without copying and pasting the code from/to different branches, but it's just a bit annoying, I wouldn't define it a big pain. Nonetheless, I fear that the creation of blocks-contrib/blocks-extra will be continuously delayed, so.......maybe you could merge it, unless you actually expect the auxiliary repos to be online soon? :)

bartvm · 2015-04-30T12:25:56Z

I'll try to do my best to get it up and running soon! So let's hope it won't end up being continuously delayed. The annoying thing about merging it is that it then becomes a copy-paste job later to move things later, breaking people's code, etc.

There's no need to copy-paste by the way. Whenever you want to use this, you can just branch master and merge plot_histogram (plus whatever other branches you need). I use that workflow quite a lot, because I've got different Blocks branches with functionality all over the place.

rizar · 2015-04-30T12:27:50Z

@fvisin , seems like you also have batch normalization PR in this one. Also I am afraid that your extension is not training resumption friendly, see __setstate__ and __getstate__ methods of the Plot extension.

Regarding the merge, I think I support @bartvm in his reluctance. For every bit of code merged in Blocks we become responsible, we have to review further PRs and bug fixes. We would much rather concentrate on the core functionality than doing that.

Cross-post!

rizar · 2015-04-30T12:32:59Z

In fact I think that creation of a non-tested repo that would simply contain bits and pieces of Blocks-compatible code is a matter of 10 minutes, and I can gladly do that. All the rest, like setup.py and other things, can be postponed: people can use PYTHONPATH to use this repo for beginning.

@bartvm, do you have anything against going this way?

bartvm · 2015-04-30T12:44:35Z

It all depends on what you want to do. If it's just a collection of code snippets, I don't think it should even go in a repo, in that case we can just add a collection of links to the wiki of this repo. If we want to set up Travis CI, Scrutinizer, write documentation, move code from here to there, etc. it becomes more than 10 minutes.

Either way, the main thing holding it back is the fact that we're supposed to create the repos as part of the new GitHub organisation, but I don't have admin rights there, so I've been unable to do anything :p I'll poke Fred/Pascal about that!

rizar · 2015-04-30T13:07:15Z

Collections of links does not do the job because one can not check out them all and import from this the checkout directory.

The whole transition is a of course a lot of work, especially moving things out from Blocks, but now I am more worried about making it possible for people to share their useful code in a convenient way.

fvisin · 2015-05-01T13:21:12Z

@bartvm thank you for the merge trick. I didn't think about it, very useful!

@rizar You are right, I was using batch normalization for a test and forgot to delete the commit. I did it now. For what concerns training resumption, Bart and I agreed on removing that part:

sub used to contain the subprocess, but you don't have it here, so the default behavior of pickle should work.

For what concerns the PR, it can wait until the extras repo will be available.

rizar · 2015-06-14T19:16:42Z

@fvisin , for your information, blocks-extras is already there!

dwf reviewed Mar 31, 2015
View reviewed changes

blocks/extensions/plot.py

If you want the server to accept connections from any address use

<ip>= 0.0.0.0.

Parameter

Copy link

Contributor

dwf Mar 31, 2015

Choose a reason for hiding this comment

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

Parameters*

dwf reviewed Mar 31, 2015
View reviewed changes

bartvm reviewed Mar 31, 2015
View reviewed changes

fvisin added 2 commits April 3, 2015 12:47

Add HistogramPlot and minor fix to Plot

82a04de

- Add HistogramPlot to plot histograms of the mean of each component of a channel - Call push only once at the beginning in Plot, afterwards it is sufficient to modify the data source for the plot to update

Plot minor enhancement

24095d4

- Visualize the right value on x axis in Plot - Show the axes's labels - Iterate only on the provided keys

rizar closed this Jun 14, 2015

dwf mentioned this pull request Aug 26, 2015

Status of Plot mega-ticket mila-iqia/blocks-extras#22

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add HistogramPlot and minor fix to Plot #569

Add HistogramPlot and minor fix to Plot #569

fvisin commented Mar 31, 2015

dwf Mar 31, 2015

fvisin commented Mar 31, 2015

dwf Mar 31, 2015

fvisin Mar 31, 2015

fvisin commented Mar 31, 2015

dwf commented Mar 31, 2015 via email

bartvm Mar 31, 2015

dwf Mar 31, 2015

fvisin Mar 31, 2015

fvisin commented Mar 31, 2015

fvisin commented Apr 14, 2015

fvisin commented Apr 29, 2015

bartvm commented Apr 29, 2015

fvisin commented Apr 30, 2015

bartvm commented Apr 30, 2015

rizar commented Apr 30, 2015

rizar commented Apr 30, 2015

bartvm commented Apr 30, 2015

rizar commented Apr 30, 2015

fvisin commented May 1, 2015

rizar commented Jun 14, 2015

Add HistogramPlot and minor fix to Plot #569

Add HistogramPlot and minor fix to Plot #569

Conversation

fvisin commented Mar 31, 2015

dwf Mar 31, 2015

Choose a reason for hiding this comment

fvisin commented Mar 31, 2015

dwf Mar 31, 2015

Choose a reason for hiding this comment

fvisin Mar 31, 2015

Choose a reason for hiding this comment

fvisin commented Mar 31, 2015

dwf commented Mar 31, 2015 via email

bartvm Mar 31, 2015

Choose a reason for hiding this comment

dwf Mar 31, 2015

Choose a reason for hiding this comment

fvisin Mar 31, 2015

Choose a reason for hiding this comment

fvisin commented Mar 31, 2015

fvisin commented Apr 14, 2015

fvisin commented Apr 29, 2015

bartvm commented Apr 29, 2015

fvisin commented Apr 30, 2015

bartvm commented Apr 30, 2015

rizar commented Apr 30, 2015

rizar commented Apr 30, 2015

bartvm commented Apr 30, 2015

rizar commented Apr 30, 2015

fvisin commented May 1, 2015

rizar commented Jun 14, 2015