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

Access to Composite PatternGenerator subobject parameters #13

Open
sf-issues opened this issue Oct 3, 2012 · 3 comments
Open

Access to Composite PatternGenerator subobject parameters #13

sf-issues opened this issue Oct 3, 2012 · 3 comments
Milestone

Comments

@sf-issues
Copy link

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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):

  •    # 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]
    
  • def _advance_pattern_generators(self,p):
    """
    Subclasses can override this method to provide constraints on
    Index: examples/lissom_fsa.ty
    --- examples/lissom_fsa.ty(revision 10157)
    +++ examples/lissom_fsa.ty(working copy)
    @@ -55,11 +55,11 @@

Input patterns

num_inputs=3
lefteye = pattern.Gaussian(aspect_ratio = 1.0, x = 1/24.0, y = 2.5/24.0,

  •                        size = 2.0 * 1.21/24.0 / 1.7, scale =0.5, offset = 0.5059)
    
  •                        size = 2.0 * 1.21/24.0 / 1.7, scale =0.5, offset = 0.5059,name='lefteye')
    

righteye = pattern.Gaussian(aspect_ratio = 1.0, x = 1/24.0, y = -2.5/24.0,

  •                        size = 2.0 * 1.21/24.0 / 1.7, scale =0.5, offset = 0.5059)
    
  •                        size = 2.0 * 1.21/24.0 / 1.7, scale =0.5, offset = 0.5059,name='righteye')
    

mouth = pattern.Gaussian(aspect_ratio = 1.0, x = -5/24.0, y = 0.0/24.0,

  •                        size = 2.0 * 1.21/24.0 / 1.7, scale =0.5, offset = 0.5059)
    
  •                        size = 2.0 * 1.21/24.0 / 1.7, scale =0.5, offset = 0.5059,name='mouth')
    

inputs=[pattern.Composite(generators = [lefteye, righteye, mouth],
operator = numpy.maximum, size = 1.7,
ceball@cloud:~/t/topographica$ ./topographica -p
cortex_density=retina_density=lgn_density=2 examples/lissom_fsa.ty -c
'print inputs[0].mouth.name'
mouth

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

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

No branches or pull requests

1 participant