ioam / topographica

A general-purpose neural simulator focusing on topographic maps.
topographica.org
BSD 3-Clause "New" or "Revised" License
53 stars 32 forks source link

Pull request: plot_view, some optimization, NumpyFile default file and GeneratorSheet.generate activity assignment #516

Closed mjabri closed 11 years ago

mjabri commented 11 years ago
jbednar commented 11 years ago

Are there other files you meant to include in this pull request? I only see the photo file.

mjabri commented 11 years ago

I re-fetched, merged and pushed.

mjabri commented 11 years ago

I push the files again (after refetching and remerging). Can you see the changes to the other files?

jbednar commented 11 years ago

No, still just the one .npy file. Try going to:

https://github.com/ioam/topographica/pull/516

and checking Files Changed; just lists one file...

The University of Edinburgh is a charitable body, registered in Scotland, with registration number SC005336.

mjabri commented 11 years ago

Ok, just checked. I think it is ok now and the changes are there.

jbednar commented 11 years ago

Thanks for making those changes. I'll be happy to accept the pull request once the above issues are fixed and once the conflicts have been addressed, which are presumably due to concurrent changes Jean-Luc has been making to activity plotting. Note that in future these changes would be much easier to handle if they are separated into independent changes to be accepted or rejected separately, as these changes are not very related to each other...

mjabri commented 11 years ago

replaced the print statement by self.warning as suggest in GeneratorSheet.generate and added comment. Also removed the plot_view statement from tiny. Changes commited and pushed. As for plot_view changes, waiting on advice on best approach to disable the viewing of sheets in Activities plot window.

jbednar commented 11 years ago

Sorry; I forgot that I already solved the problem of controlling which plots are printed, long ago! A TemplatePlotGroup (like Activity, Orientation Preference, and so on) has a parameter named filterfn that does what you want:

filterfn = param.Callable(default=alwaystrue,doc="""
    Boolean function allowing control over which items will be plotted.
    E.g.: filterfn=lambda x: x.name in ['Retina','V1'] for a plot ranging over
    Sheets, or filterfn=lambda x: x[0]==x[1] for a plot ranging over coordinates.""")

Possible filterfns you might want to use could be:

filterfn=lambda x: in ['Retina','V1']
filterfn=lambda x: x.hasattr("plot_view")

(assuming you did topo.sim.V1.plot_view=True for all the ones you wanted). The criterion can be anything you like, really; any way that you wish to specify which ones to use. You can set the filterfn at the class level, i.e.:

import topo.plotting.plotgroup
topo.plotting.plotgroup.TemplatePlotGroup.filterfn=lambda x: x.hasattr("plot_view")

to apply to all plotgroups, or you can set it on a specific plotgroup instance (e.g. the Activity plot) if you want it the changes to apply to a single plotgroup only.

If this solves your problem, please think of where in the documentation we should have mentioned this, and submit a patch for the .rst file to help the next person find it more easily.

mjabri commented 11 years ago

Yes, filterfn looks good. Thanks.

jbednar commented 11 years ago

It looks good now, thanks. There are still extraneous things that I don't want to apply, such as the submodule changes and changes to tiny.ty, so I'll need to go through bit by bit and only take the good bits, so I can't just hit "Merge pull request". I should be able to do that on Monday. Note that as far as I know we don't need to do the plot_view parameter definition any more either, so I won't take that bit either. Not defining plot_view as a Parameter makes it a bit more awkward to set the plot_view value on objects (i.e., you can't put plot_view=True into the constructor without getting a warning, and instead have to set it after the object is created), but it doesn't restrict the actual functionality. Sound ok?

mjabri commented 11 years ago

Yes, sounds good.

jbednar commented 11 years ago

Marwan, can you clarify the copyright status of the numpy_array_feret_photo.npy? All contributions to Topographica need to be freely redistributable under a BSD license, and I couldn't find any explicit copyright or license statement on the FERET website. If you can't find it, maybe you can take one of my public-domain photos from http://doozy.inf.ed.ac.uk/jbednar/album and save that in Numpy format instead?

jbednar commented 11 years ago

Ok, I've merged all that I can, and am waiting on clarification of the license status of the .npy file and of the specific case where a PatternGenerator can return None, before I can merge those last two bits.

mjabri commented 11 years ago

Actually i also searched and could not find any explicit statement. I will ask NIST.

On Mon, May 27, 2013 at 7:23 AM, James A. Bednar notifications@github.comwrote:

Marwan, can you clarify the copyright status of the numpy_array_feret_photo.npy? All contributions to Topographica need to be freely redistributable under a BSD license, and I couldn't find any explicit copyright or license statement on the FERET website.

— Reply to this email directly or view it on GitHubhttps://github.com/ioam/topographica/pull/516#issuecomment-18493955 .

jbednar commented 11 years ago

Can we close this pull request now?