search

holoviz-topics / imagen

ImaGen: Generic Python library for 0D, 1D and 2D pattern distributions
https://imagen.holoviz.org/
BSD 3-Clause "New" or "Revised" License
31 stars 16 forks source link

Access to Composite PatternGenerator subobject parameters #13

Open sf-issues opened 12 years ago

sf-issues commented 12 years ago

Converted from SourceForge issue 2144996, submitted by jbednar Submit Date: 2008-10-03 20:11 GMT

Classes like SineGratingDisk, SineGratingRectangle, SineGratingRing, are redundant, because a Composite can easily do any of those things, but they have proliferated because a Composite does not make it easy to control the parameters of the underlying object. The special-purpose classes define various parameters at the top level, and then (in principle) apply those to the underlying pattern. That way the patterns can be used in the Test Pattern window, and (even more importantly) for measuring tuning curves and maps. We would all like a much more general way to define such patterns, so that new patterns don't require writing a new, complicated class.

For the very common special case of a pattern with a mask of some shape, a simple approach is to use the ConnectionField mask. I added code today to make that work well, and now e.g. SineGratingDisk can be written as simply: 

class SineGratingDisk(SineGrating):
    """2D sine grating pattern generator with a circular mask."""
    mask_shape = param\.Parameter(default=Disk(smoothing=0))

or even SineGrating(mask_shape=Disk(smoothing=0)) (without needing a new class, though this version won't show up in the Test Pattern window). This takes care of SineGratingDisk, SineGratingRectangle, SineGratingRing, and any other pattern masked by a simple shape.

However, it doesn't help the general case of controlling the subpatterns of a Composite. E.g. GaussianCloud doesn't fit, because we typically use that for ConnectionField patterns, and CFProjections override the masks to make them circular or some other shape. OrientationContrastPattern consists of two concentric rings, and thus isn't just a masked pattern either.

As a first step, I've cleaned up GaussianCloud as an example of how those classes can work best now (e.g. r9389), by inheriting from Composite and passing the value of a few specially defined parameters down to subobjects. This approach works well, but it does require defining a new class and new parameters, rather than just passing some parameters to Composite.

To solve the problem completely, I think this plan would work:

  1. Change Composite.generators into a dictionary, instead of a list, so that every subobject has a name. For backwards compatibility, the Composite constructor would probably need to continue accepting a list, but then generate arbitrary unique names (obj1, obj2, etc.) and put them into a dictionary. New code would then use a dictionary, with names for each object.

  2. Add a method to Composite to shadow the parameters of an underlying object in a general way, e.g. parameter "orientation" of object "sine" would show up as parameter "sine.orientation" or maybe "sine:orientation". Two ways this could work:

    a) Like TkParameterizedBase, i.e. automatically mapping everything in an underlying object into the Composite, using the special names as above. The advantage of this is is that any Composite would work automatically. However, the list of parameters will quickly get quite huge, with only a few of them actually useful to expose at the top level, which will make the Test Pattern window nearly unusable.

    b) We could add method for promoting only specific subobject parameters to the top level, potentially allowing shorter names (e.g. 'frequency' could be promoted as 'frequency' rather than 'sine.frequency', because Composite doesn't have any frequency parameter to conflict. However, this requires specific work to make the subobject parameter available at the Composite level, and I don't know if that's appropriate.

There may also be some middle ground between these two.

In any case, this all seems feasible, and doesn't require e.g. parsing any special string languages as I had originally feared. (Even though parameter names are like "sine.frequency", we don't need to break it into "sine" and "frequency", because the mapping mechanism should make the subobject parameter show up as a Composite parameter that just happens to be named "sine.frequency", and so we don't need to do any further text processing.

If anyone wants to work on this, please let me know. It seems pretty concrete now; it used to seem too vague even to attempt...

sf-issues commented 12 years ago

Submitted by jbednar Date: 2008-10-05 08:07 GMT

We've now made it easier to edit mask_shape in the Test Pattern window; one can simply select any PatternGenerator and then right click to change its parameters. So the GUI should now support masked patterns very well. Arbitrary composite patterns, however, still require a new class with special mapping between its parameters and those of its subobjects.

sf-issues commented 12 years ago

Submitted by ceball Date: 2009-03-24 15:52 GMT

Could we go with the idea of 'promoting' parameters? In Composite:

def init(self,**params):

code to create the 'promoted' dictionary

def getattr(self,name):

only called if name isn't an attribute of self

   return getattr(self.promoted[name],name)

def setattr(self,name,val): if name in self.promoted: setattr(self.promoted[name],name,val) else:

normal setattr

       super(Composite,self).__setattr__(name,val)

def promote_attr(self,name,obj): self.promoted[name]=obj

Code like that would allows the following:

ceball@cloud:~/t/topographica$ ./topographica -i -p cortex_density=retina_density=lgn_density=2 examples/lissom_fsa.ty ... topo_t000000.00_c1>>> inputs[0].promote_attr('aspect_ratio',inputs[0].generators[2])

topo_t000000.00_c2>>> inputs[0].aspect_ratio Out[2]:1.0

topo_t000000.00_c3>>> inputs[0].aspect_ratio=2.0

topo_t000000.00_c4>>> inputs[0].generators[2].aspect_ratio Out[4]:2.0

topo_t000000.00_c5>>> inputs[0].generators[2].aspect_ratio=3

topo_t000000.00_c6>>> inputs[0].aspect_ratio Out[6]:3

(We'd also have to do something for the params() method, which e.g. the gui uses to determine what to display on properties frames.)

sf-issues commented 12 years ago

Submitted by nobody Date: 2010-07-06 22:07 GMT

(Added email discussion; still haven't resolved the issues)

From: C. E. Ball Date: Mar 15 19:53:35 2009 +0000

On Mon, Mar 9, 2009 at 00:45, James A. Bednar jbednar@inf\.ed\.ac\.uk wrote:

Do you think it would make sense for a Composite PatternGenerator to support the [] operator, to give access to the underlying generators for the purpose of setting parameters? Or, better, to use the generators names to support . notation?

Seems like it would be convenient, and I support the . notation in this case. Since the name of the subpattern is being used, there isn't the confusion of tkparameterizedbase (where self.x could give you x from self or from the shadowed PO).

I don't, however, see how this will solve our original problem ('promoting' parameters of subpatterns). If I have a Composite pattern c with several generators, one of which has frequency, when e.g. PatternPresenter comes to set the frequency, it still won't find 'frequency' as an attribute of c. Still, having c.subpattern_name.frequency work is good for users, though.

I tried adding . support to Composite:

def getattr(self,name):

Provide 'dot' access to entries in the generators list

   gs = [g for g in self.generators if g.name==name]
   assert len(gs)==1
   return gs[0]

But that doesn't seem to work. I think you actually understand how such attribute lookup works far better than I do; do you think this is feasible and/or a good idea?

I don't see a problem with that code (except that the assert should be replaced with an error for the list being length 0 - i.e. "name isn't an attribute of self or any of the generators" - and for it being greater than 1 - i.e. "more than one subpattern has the attribute 'name'").

In fact, trying it now, I find it works:

ceball@cloud:~/t/topographica$ svn diff Index: topo/pattern/basic.py --- topo/pattern/basic.py(revision 10157) +++ topo/pattern/basic.py(working copy) @@ -568,6 +568,13 @@ size = param.Number(default=1.0,doc="Scaling factor applied to all sub-patterns.")

+ def getattr(self,name):

This reminded me that we still want to deal with the general problem. I finally read your feature request [https://sourceforge.net/tracker/index.php?func=detail&aid=2144996&group_id=53602&atid=470932]. I don't understand how naming the subpatterns will help things like PatternPresenter to set e.g. the frequency of a composite's sine grating. How would it know the right name to use? Would the features list contain 'sine.frequency' rather than just 'frequency'? Have I just totally missed something about your suggestion?

Could we go with the idea of 'promoting' parameters? In Composite:

def \_\_init\_\_\(self,\*\*params\):
  \# code to create the 'promoted' dictionary

def \_\_getattr\_\_\(self,name\):
    \# only called if name isn't an attribute of self
    return getattr\(self\.promoted\[name\],name\)

def \_\_setattr\_\_\(self,name,val\):
    if name in self\.promoted:
        setattr\(self\.promoted\[name\],name,val\)
    else:
        \# normal setattr
        super\(Composite,self\)\.\_\_setattr\_\_\(name,val\)

def promote\_attr\(self,name,obj\):
    self\.promoted\[name\]=obj

Ignoring the earlier changes for named subpatterns, code like this allows the following:

ceball@cloud:~/t/topographica$ ./topographica -i -p cortex_density=retina_density=lgn_density=2 examples/lissom_fsa.ty ... topo_t000000.00_c1>>> inputs[0].promote_attr('aspect_ratio',inputs[0].generators[2])

topo_t000000.00_c2>>> inputs[0].aspect_ratio Out[2]:1.0

topo_t000000.00_c3>>> inputs[0].aspect_ratio=2.0

topo_t000000.00_c4>>> inputs[0].generators[2].aspect_ratio Out[4]:2.0

topo_t000000.00_c5>>> inputs[0].generators[2].aspect_ratio=3

topo_t000000.00_c6>>> inputs[0].aspect_ratio Out[6]:3

(We'd also have to do something for the params() method, which e.g. the gui uses to determine what to display on properties frames.)

Have I missed something?

'Automatic promotion' (i.e. no promote_param(), instead finding the appropriate attribute automatically) could maybe happen, but I don't think I'd like to see that happen with 'dot' notation (as is done in TkParameterizedBase). Maybe using [] notation might be ok, so that dot is reserved for definitely acting on the object, whereas [] means 'from somewhere'! Anyway, before thinking about that we should decide if it's better or not to require that parameter promotions be explicit.

Chris