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

Sparse and Dense Noise #19

Closed h-mayorquin closed 9 years ago

h-mayorquin commented 10 years ago

I am a master student working with Jan Antolik in CNRS at Gif Sur Yvette, France. I added functions that are able to produce sparse and dense noise in the class random. It is my first use of git so my apologies if I failed to comply with any style or programming guidance that I was not aware of.

ceball commented 10 years ago

Hi

Thanks for contributing!

I can make a few quick comments about coding conventions/style (I'll comment on the diff). Unfortunately I don't currently have time to try out the patters or to study the code.

Chris

ceball commented 10 years ago

By the way, from reading the docstrings it wasn't clear to me what these classes would generate (e.g. I didn't understand what grid_size is supposed to control).

h-mayorquin commented 10 years ago

I implemented your comments and answer to your diff. I also extended a little bit the docstring to provide an example that hopefully will clarify what grid_size controls in each case. The new commit and push is up.

jbednar commented 10 years ago

Apart from Chris's good suggestions above, there are a few issues:

  1. The whitespace changes to surrounding code should be reverted.
  2. I think grid_size should be redefined as grid_density instead, so that it specifies the number of grid elements per sheet coordinate, rather than a single fixed size. This will make the results be independent of edge buffering, make them scalable when the area being simulated changes, and would also help eliminate the restriction to square matrices.
  3. Is there a good reason to have it produce values -1, 0, 1, unlike all the other PatternGenerators that are in the range [0,1]? Seems like it ought to be [0,0.5,1] by default so that we can interpret it as a visual stimulus (where negative intensities aren't possible).
  4. I'm not sure about the name of DenseNoise. "DenseNoise" as a description could just as well apply to imagen.UniformRandom; the main differences from that class are that this one is drawn on a grid whose density might be smaller than the GeneratorSheet's density, and that it's been discretized to one of three possible values. "DenseNoise" doesn't convey either of those things to me, but if there is widespread agreement in the literature that dense noise does mean a grid whose elements have one of three values, then it's fine. Otherwise, maybe "NoiseGrid"?
  5. The code for DenseNoise N%n!=0 seems very complex; can't the smaller grid simply be scaled up automatically using some already available Numpy function? Maybe the required function is in SciPy, though, in which case we can't assume it's available.
  6. Please consider whether you could get the results that you want using a combination of lower-level components. E.g. there could be a Discretize() TransferFn (something we were just discussing for other reasons) to map a range 0..1 into 0, 0.5,or 1 (generalizing BinaryThreshold to any number of values). And there could be a Grid PatternGenerator that takes any other PatternGenerator (e.g. UniformRandom(output_fns=[transferfn.Discretize(bins=3)) by default) with one density, and scales it up spatially to another density instead. The main reason to do this is if different studies use slightly different definitions of DenseNoise, and you'd then be able to handle all of them. Up to you.
  7. Can't SparseNoise with grid=False already be implemented by RawRectangle with random x, y, and scale?
  8. And then SparseNoise with grid=True could be implemented either by putting a modulo operator on x and y for a RawRectangle, or by using the same RawRectangle combined with Grid. Again, up to you.

Those last few suggestions are optional, but please consider them. Once you have, and have addressed the ones above it, I'd be happy to merge the pull request. Thanks for contributing!

Jim

jbednar commented 10 years ago

Jean-Luc suggests making the set of discrete levels be configurable, as in values = param.List(default=[0.0,0.5,1.0]) so that someone could use [0.0,1.0] or any other discrete set of values if they want.

I think this suggestion goes well with my suggestion for making a general-purpose Grid pattern generator, which Jean-Luc also supports. I.e., instead of DenseNoise, you can implement a simple RandomChoice PatternGenerator, which is just like UniformRandom but instead of choosing values from an interval, it chooses them from a predefined list of choices (values=[0,0.5,1.0] by default). You can then easily combine a RandomChoice PatternGenerator with a Grid, to get the pattern you want, but then RandomChoice will be available for anyone who wants that behavior, and any PatternGenerator will be able to be made into a Grid, so that the result is much more powerful than simply offering a DenseGrid.

Jean-Luc points out that Grid would go into imagen/__init__.py, not imagen/random.py, since it has nothing to do with randomness. He also doesn't like the name Grid, since the actual operation being performed is a box filter or a spatial upsampling, and suggests NearestSampling, BoxSampling, BoxInterpolation, or NearestInterpolation as a better name for Grid. Me, I don't mind Grid, but maybe BoxScaling might be good.

h-mayorquin commented 10 years ago

Hi, sorry for not answering before. I have been very busy here in the lab.

I will denote the the answers to the points by Jbednar with A


Apart from Chris's good suggestions above, there are a few issues:

The whitespace changes to surrounding code should be reverted. A: Done

I think grid_size should be redefined as grid_density instead, so that it specifies the number of grid elements per sheet coordinate, rather than a single fixed size. This will make the results be independent of edge buffering, make them scalable when the area being simulated changes, and would also help eliminate the restriction to square matrices.

A: We find that for the user it is more practical to specify the grid size, which is the parameter one will typically get in experimental paper, and let imagen recalculate the rest of the parameters.

Is there a good reason to have it produce values -1, 0, 1, unlike all the other PatternGenerators that are in the range [0,1]? Seems like it ought to be [0,0.5,1] by default so that we can interpret it as a visual stimulus (where negative intensities aren't possible).

A: That is already fixed, by puting defaults value of scales and offset in the class.

I'm not sure about the name of DenseNoise. "DenseNoise" as a description could just as well apply to imagen.UniformRandom; the main differences from that class are that this one is drawn on a grid whose density might be smaller than the GeneratorSheet's density, and that it's been discretized to one of three possible values. "DenseNoise" doesn't convey either of those things to me, but if there is widespread agreement in the literature that dense noise does mean a grid whose elements have one of three values, then it's fine. Otherwise, maybe "NoiseGrid"?

A: We are following the naming in literature.

The code for DenseNoise N%n!=0 seems very complex; can't the smaller grid simply be scaled up automatically using some already available Numpy function? Maybe the required function is in SciPy, though, in which case we can't assume it's available.

A: A: Indeed I agree, it is more complex that I would like. I am looking for a simpler one but I haven't found it so far. if I come across a smart or more elegant way to do it I commit myself to change it.

Please consider whether you could get the results that you want using a combination of lower-level components. E.g. there could be a Discretize() TransferFn (something we were just discussing for other reasons) to map a range 0..1 into 0, 0.5,or 1 (generalizing BinaryThreshold to any number of values). And there could be a Grid PatternGenerator that takes any other PatternGenerator (e.g. UniformRandom(output_fns=[transferfn.Discretize(bins=3)) by default) with one density, and scales it up spatially to another density instead. The main reason to do this is if different studies use slightly different definitions of DenseNoise, and you'd then be able to handle all of them. Up to you.

A: This indeed seems like a more general and indeed more clean approach. However here I am limited by the time constrains of my say here and I don't have time to work in this project fully. I in the future the function is implemented I will surely like to remove or adapt the function to the new scheme.

Can't SparseNoise with grid=False already be implemented by RawRectangle with random x, y, and scale?

And then SparseNoise with grid=True could be implemented either by putting a modulo operator on x and y for a RawRectangle, or by using the same RawRectangle combined with Grid. Again, up to you.

A: this is the answer to seven too. Yes, indeed I have been playing around after your suggestion and indeed it seems that I could be done that way. However I would prefer to have one class that can produce both types of noise with just a change of parameter

Those last few suggestions are optional, but please consider them. Once you have, and have addressed the ones above it, I'd be happy to merge the pull request. Thanks for contributing!

Jim


Sorry again for the late response.

jbednar commented 10 years ago

Thanks for fixing most of the whitespace issues. I'm happy with most of what you say above, i.e. you can keep calling it DenseNoise and SparseNoise if you prefer, you can keep the current monolithic approach if making a more flexible version is too time consuming for now, and the complex code for DenseNoise N%n!=0 is ok if you can't see a simpler version.

But I've put comments around your recent changes to get [0,0.5,1], because they interfere with the usual scale and offset mechanisms that we supply for the users to be able to manipulate all ImaGen patterns. I.e., scale=1.0 and offset=0.0 for all of our patterns, so that someone can set scale=2.0 and be sure they will get a pattern twice as bright as normal. Your recent changes make that convention no longer work. Instead, the values should be left at their defaults, and the surrounding code should calculate the right values as appropriate to result in [0,0.5,1]. This should require less code (as you no longer need to re-declare all those parameters), and is necessary for these patterns to work well for the users.

Finally, I appreciate that you find it easier to specify the grid size, but we cannot allow that, because it causes too many problems. The reason is that if the user is simulating a sheet size of 1.0x1.0, then it is just as easy for them to set the grid_density to the value specified in the experimental paper as it is for them to set grid_size to that same value. And if the user is not using a sheet size of 1.0x1.0, the result from using grid_size will be bogus anyway, if their size is not square or if their size is meant to be scaled up or down from the original experiment (which is how we normally use the sheet size). There's no meaning for letting imagen calculate the rest of the parameters from the grid_size -- the grid_size doesn't give imagen the information it needs to do that; only the grid_density does. So even though grid_size might seem like it helps the user, it actually hurts them. If you don't want to make this change, that's fine, but I'll have to make it after accepting the code anyway, so that it fits with the assumptions and structure of ImaGen, so it's cleaner if you do it.

Again, thanks for submitting this, and I assure you that it's much easier to get the second code submission accepted than the first! :-) And I do think these are useful classes, or else we wouldn't be bothering to try to make them work well.

h-mayorquin commented 10 years ago

Hi, I fix the white-spaces issue and also change the default behavior to work appropriately. They are working fine according to my testing.

Regarding changing grid_size for grid_density I just have a question to see if I understand what you mean:

In that setting grid_size = grid_density * sheet size, right?

With that information I will be able to implement the changes.

Kind regards

jbednar commented 10 years ago

Right; grid_x_size = grid_density * sheet_x_size and grid_y_size = grid_density * sheet_y_size.

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

h-mayorquin commented 10 years ago

Grid density instead of grid size is now added.

jbednar commented 10 years ago

Sorry for the delay! Given the number of commits compared to the size of these classes, I've just taken the final version of them and committed it to my local repository. I then did a cleanup pass on it, with the commit message "Cleaned up docstrings and formatting. GUI-related fixes: hid unused parameters, added lower bound on grid_density, added softbounds, changed asserts to warnings. Removed stray print statement". I've verified that the examples provided in the docstrings work (though I had to update one of them that had bogus output), and for some combinations of parameters the patterns clearly work well.

However, I get a slew of errors for any other combination of parameters, so I think there are still a lot of problems, and I haven't yet pushed this code to the main repository. I've attached my version of it below, to be put into external/imagen/imagen/random.py. If I do that and run Topographica using:

  ./topographica -g -p cortex_density=2 -p retina_density=48 examples/gcal.ty  -c 'topo.guimain["Simulation"]["Test Pattern"]()'

and select either DenseNoise or SparseNoise as the pattern generator, then vary the scale or offset or grid parameters, everything looks good. If I instead vary the grid_density parameter, I get huge lists of errors for nearly all values, apparently only getting reasonable results for the cases that fit neatly into the underlying pattern grid. Given all the code in there to work with cases where the mapping doesn't match, I would have thought that all such grid sizes were supported. In any case, the code should either support all reasonable grid_density values, or else it should "snap" the value to the nearest supported value, regardless of what it is set to. There's an example of the latter in there already now, which snaps the grid_density to a maximum of the lower of xdensity and ydensity, but clearly that's not enough to prevent all the errors. So, please remove these errors, either by avoiding them or making the code work for them.

BTW, I only tried the above example with -p retina_density=48, but once it works for that case, it should be tried for other retina_density values just in case, e.g. odd numbers like 49, prime numbers like 47, etc., each of which could reveal other problems.

Thanks!

class DenseNoise(RandomGenerator):
    """
    2D Dense noise pattern generator, constrained to a grid.

    Similar to UniformRandom, but draws the noise pattern in a grid
    that can be smaller than the actual density of the
    PatternGenerator.

    By default, this produces a matrix with random values 0.0, 0.5 and 1.
    When a scale and an offset are provided the transformation maps them to:

     0 -> offset
     0.5 -> offset + 0.5 * scale
     1 -> offset + scale

    --------
    Examples
    --------

    DenseNoise(grid_density=1, bounds=BoundingBox(radius=1),
        xdensity=4, ydensity=4)  will produce something like this:

    [[ 1.  1.  1.  1.  0.  0.  0.  0. ]
     [ 1.  1.  1.  1.  0.  0.  0.  0. ]  
     [ 1.  1.  1.  1.  0.  0.  0.  0. ]  Here the Sheet-coordinate size is 2.0x2.0,
     [ 1.  1.  1.  1.  0.  0.  0.  0. ]  so grid_density=1 yields a 2x2 grid
     [ 0.  0.  0.  0.  0.5 0.5 0.5 0.5]  sampled at 4 units per grid cell
     [ 0.  0.  0.  0.  0.5 0.5 0.5 0.5]
     [ 0.  0.  0.  0.  0.5 0.5 0.5 0.5]
     [ 0.  0.  0.  0.  0.5 0.5 0.5 0.5]])

    DenseNoise(grid_density=2, bounds=BoundingBox(radius=1),
        xdensity=4, ydensity=4)  on the other hand will produce something like:

    [[ 1.  1.  0.  0.  0.  0.  0.5 0.5]
     [ 1.  1.  0.  0.  0.  0.  0.5 0.5]
     [ 1.  1.  1.  1.  0.  0.  0.  0. ]  Again the Sheet-coordinate size is 2.0x2.0,
     [ 1.  1.  1.  1.  0.  0.  0.  0. ]  but grid_density=2 yields a 4x4 grid
     [ 0.  0.  0.5 0.5 1.  1.  1.  1. ]  with 2 units per grid cell
     [ 0.  0.  0.5 0.5 1.  1.  1.  1. ]
     [ 1.  1.  0.  0.  1.  1.  1.  1. ]
     [ 1.  1.  0.  0.  1.  1.  1.  1. ]]

    -----
    Notes
    -----

    1. This method works much faster when the noise matrix falls neatly
       into the pixel matrix (~100 times faster)

    2. The value of each pixel in the generated pattern is determined
       by where the center of that pixel lies in the underlying grid,
       regardless of any overlap of that pixel with other grid
       squares.

    3. If a particular number of cells N is wanted, divide it by the
       length of the side of the bounding box to determine the
       grid_density. For example, if the user wants to have N=10 cells
       for a BoundingBox(radius=1) (which gives a bounding box size of
       2.0x2.0), the grid_density must be set to N/2 = 5 in order to
       have ten cells.  

    4. The xdensity and ydensity must both be at least as large as the
       grid_density, e.g. 5 for the above example.
    """

    grid_density = param.Number(default=1, bounds=(1,None), softbounds=(1,50), doc="""
        Grid elements per 1.0 distance in Sheet coordinates.""")

    # Hide unused parameters
    x = param.Number(precedence=-1)
    y = param.Number(precedence=-1)
    size = param.Number(precedence=-1)

    def _distrib(self, shape, p):
        max_density = min(p.xdensity,p.ydensity)
        if (p.grid_density > max_density):
            self.warning("Requested grid_density %s larger than xdensity %s or ydensity %s; capped at %s" % 
                         (p.grid_density,p.xdensity,p.ydensity,max_density))
            p.grid_density = max_density

        Nx = shape[1]
        Ny = shape[0] # Size of the pixel matrix

        SC = SheetCoordinateSystem(p.bounds, p.xdensity, p.ydensity)
        unitary_distance_x = SC._SheetCoordinateSystem__xstep
        unitary_distance_y = SC._SheetCoordinateSystem__ystep

        sheet_x_size = round(unitary_distance_x * Nx)
        sheet_y_size = round(unitary_distance_y * Ny)

        # Sizes of the structure matrix
        nx = int(sheet_x_size * p.grid_density)
        ny = int(sheet_y_size * p.grid_density)

        ps_x = int(round(Nx / nx)) #Closest integer
        ps_y = int(round(Ny / ny))

        # If the noise grid is proportional to the pixel grid
        # and fits neatly into it then this method is ~100 times faster
        if ( Nx % nx == 0) and (Ny % ny == 0):

            if (ps_x == 1) and (ps_y == 1):  #This is faster to call the whole procedure
                result = 0.5 * (p.random_generator.randint(-1, 2, shape) + 1)
                return  result * p.scale + p.offset

            else:
                # This is the actual matrix of the pixels
                A = np.zeros(shape)
                # Noise matrix that contains the structure of 0, 0.5 and 1's
                Z = 0.5 * (p.random_generator.randint(-1, 2, (nx, ny)) + 1 )

                # Noise matrix is mapped to the pixel matrix
                for i in range(nx):
                    for j in range(ny):
                        A[i * ps_y: (i + 1) * ps_y, j * ps_x: (j + 1) * ps_x] = Z[i,j]

                return A * p.scale + p.offset

        # General method in case the noise grid does not
        # fall neatly in the pixel grid
        else:

            x_points,y_points = SC.sheetcoordinates_of_matrixidx()

            # Obtain length of the side and length of the
            # division line between the grid

            division_x = sheet_x_size / nx
            division_y = sheet_y_size / ny

            # This is the actual matrix of the pixels
            A = np.zeros(shape)
            # Noise matrix that contains the structure of 0, 0.5, and 1's
            Z = 0.5 * (p.random_generator.randint(-1, 2, (nx, ny)) + 1 )

            # Noise matrix is mapped to the pixel matrix
            for i in range(Nx):
                for j in range(Ny):
                    # Map along the x coordinates
                    aux1 = x_points[i] + (sheet_x_size  / 2)
                    outcome1 = int(aux1 / division_x)
                    # Map along the y coordinates
                    aux2 = y_points[-(j+1)] + (sheet_y_size  / 2)
                    outcome2 = int(aux2 / division_y)
                    # Assign
                    A[j][i] = Z[outcome2][outcome1]

            return A * p.scale + p.offset

class SparseNoise(RandomGenerator):
    """
    2D sparse noise pattern generator, with optional constraining to a grid

    Draws a square pixel of random brightness at a random location,
    either entirely random on the pattern surface or chosen from a
    predefined grid of possible positions.

    By default, produces a matrix with 0.5 everywhere except for a
    square patch in one random location. This value is randomly
    assigned to either 0 or 1, and then is scaled with the parameters
    scale and offset in the following way:

     0 -> offset
     1 -> offset + scale

    --------
    Examples
    --------

    SparseNoise(grid_density=1, grid=True, bounds=BoundingBox(radius=1),
        xdensity=4, ydensity=4) will produce something like this:

    [[ 0.5 0.5 0.5 0.5 0.  0.  0.  0. ] 
     [ 0.5 0.5 0.5 0.5 0.  0.  0.  0. ] 
     [ 0.5 0.5 0.5 0.5 0.  0.  0.  0. ]  Here the Sheet-coordinate size is 2.0x2.0,
     [ 0.5 0.5 0.5 0.5 0.  0.  0.  0. ]  so grid_density=1 yields a 2x2 grid
     [ 0.5 0.5 0.5 0.5 0.5 0.5 0.5 0.5]  sampled at 4 units per grid cell, with 0.5
     [ 0.5 0.5 0.5 0.5 0.5 0.5 0.5 0.5]  everywhere except the one active cell
     [ 0.5 0.5 0.5 0.5 0.5 0.5 0.5 0.5]
     [ 0.5 0.5 0.5 0.5 0.5 0.5 0.5 0.5]]

    SparseNoise(grid_density=2, grid=True, bounds=BoundingBox(radius=1),
        xdensity=4, ydensity=4) on the other hand will produce something like:

    [[ 0.5 0.5 0.5 0.5 0.5 0.5 0.5 0.5]
     [ 0.5 0.5 0.5 0.5 0.5 0.5 0.5 0.5]
     [ 0.5 0.5 0.5 0.5 0.5 0.5 0.5 0.5]  Again the Sheet-coordinate size is 2.0x2.0,
     [ 0.5 0.5 0.5 0.5 0.5 0.5 0.5 0.5]  but grid_density=2 yields a 4x4 grid
     [ 0.5 0.5 0.5 0.5 0.5 0.5 1.  1. ]  with 2 units per grid cell
     [ 0.5 0.5 0.5 0.5 0.5 0.5 1.  1. ]  
     [ 0.5 0.5 0.5 0.5 0.5 0.5 0.5 0.5]
     [ 0.5 0.5 0.5 0.5 0.5 0.5 0.5 0.5]]

    SparseNoise(grid_density=2, grid=False, bounds=BoundingBox(radius=1),
        xdensity=4, ydensity=4) will produce something like:

    [[ 0.5 0.5 0.5 0.5 0.5 0.5 0.5 0.5]
     [ 0.5 0.5 0.5 0.5 0.5 0.5 0.5 0.5]
     [ 0.5 0.5 0.5 0.5 0.5 0.5 0.5 0.5]
     [ 0.5 0.5 0.5 0.5 0.5 0.5 0.5 0.5]  Here notice that the patch is no longer
     [ 0.5 0.5 0.5 0.5 0.5 0.  0.  0.5]  aligned with a fixed grid
     [ 0.5 0.5 0.5 0.5 0.5 0.  0.  0.5]
     [ 0.5 0.5 0.5 0.5 0.5 0.5 0.5 0.5]
     [ 0.5 0.5 0.5 0.5 0.5 0.5 0.5 0.5]]

    -----
    Notes
    -----

    1. This method works ~100 times faster when the noise matrix falls neatly
       into the pixel matrix

    2. The value of each pixel in the generated pattern is determined
       by where the center of that pixel lies in the underlying grid,
       regardless of any overlap of that pixel with other grid
       squares.

    3. If a particular number of cells N is wanted, divide it by the
       length of the side of the bounding box to determine the
       grid_density. For example, if the user wants to have N=10 cells
       for a BoundingBox(radius=1) (which gives a bounding box size of
       2.0x2.0), the grid_density must be set to N/2 = 5 in order to
       have ten cells.  

    4. The xdensity and ydensity must both be at least as large as the
       grid_density, e.g. 5 for the above example.
    """

    grid_density = param.Number(default=1, bounds=(1,None), softbounds=(1,50), doc="""
        Grid elements per 1.0 distance in Sheet coordinates.""")

    grid = param.Boolean(default=True, doc="""
        If True, each spot is snapped to a grid, so that subsequent
        spots are forced to overlap either entirely or not at all,
        never partially.  Otherwise, the spot size is fixed by the
        grid_density, but it may appear anywhere.""")

    # Hide unused parameters
    x = param.Number(precedence=-1)
    y = param.Number(precedence=-1)
    size = param.Number(precedence=-1)

    def _distrib(self, shape, p):
        max_density = min(p.xdensity,p.ydensity)
        if (p.grid_density > max_density):
            self.warning("Requested grid_density %s larger than xdensity %s or ydensity %s; capped at %s" % 
                         (p.grid_density,p.xdensity,p.ydensity,max_density))
            p.grid_density = max_density

        Nx = shape[1]
        Ny = shape[0] # Size of the pixel matrix

        SC = SheetCoordinateSystem(p.bounds, p.xdensity, p.ydensity)
        unitary_distance_x = SC._SheetCoordinateSystem__xstep
        unitary_distance_y = SC._SheetCoordinateSystem__ystep

        sheet_x_size = round(unitary_distance_x * Nx)
        sheet_y_size = round(unitary_distance_y * Ny)

        # Sizes of the structure matrix
        nx = int(sheet_x_size * p.grid_density)
        ny = int(sheet_y_size * p.grid_density)

        ps_x = int(round(Nx / nx)) #Closest integer
        ps_y = int(round(Ny / ny))

        # This is the actual matrix of the pixels
        A = np.ones(shape) * 0.5

        if p.grid == False:  #The centers of the spots are randomly distributed in space

            x = p.random_generator.randint(0, Nx - ps_x + 1)
            y = p.random_generator.randint(0, Ny - ps_y + 1)
            z = p.random_generator.randint(0,2)

            # Noise matrix is mapped to the pixel matrix
            A[x: (x + ps_y), y: (y + ps_x)] =  z

            return A * p.scale + p.offset

        else: #In case you want the grid

            if  ( Nx % nx == 0) and (Ny % ny == 0): #When the noise grid falls neatly into the the pixel grid
                x = p.random_generator.randint(0, nx)
                y = p.random_generator.randint(0, ny)
                z = p.random_generator.randint(0,2)

               # Noise matrix is mapped to the pixel matrix (faster method)
                A[x*ps_y: (x*ps_y + ps_y), y*ps_x: (y*ps_x + ps_x)] = z

                return A * p.scale + p.offset

            else: # If noise grid does not fit neatly in the pixel grid (slow method)

                x_points,y_points = SC.sheetcoordinates_of_matrixidx()

                # Obtain length of the side and length of the
                # division line between the grid

                division_x = sheet_x_size / nx
                division_y = sheet_y_size / ny

                # Construct the noise matrix
                Z = np.ones((nx,ny)) * 0.5
                x = p.random_generator.randint(0, nx)
                y = p.random_generator.randint(0, ny)
                z = p.random_generator.randint(0,2)
                Z[x,y] = z

                # Noise matrix is mapped to the pixel matrix
                for i in range(Nx):
                    for j in range(Ny):
                        # Map along the x coordinates
                        aux1 = x_points[i] + (sheet_x_size / 2)
                        outcome1 = int(aux1 / division_x)
                        # Map along the y coordinates
                        aux2 = y_points[-(j+1)] + (sheet_y_size / 2)
                        outcome2 = int(aux2 / division_y)
                        # Assign
                        A[j][i] = Z[outcome2][outcome1]

                return A * p.scale + p.offset
jbednar commented 10 years ago

Have you had a chance to try to fix the errors so that this code can be accepted? I'll be happy to merge it in once it works!

h-mayorquin commented 10 years ago

Hi, James. I have been very busy finishing my M2 project. As soon as I am done with this, in a week or two, I will do the required changes.

All the best,

On Thu, Jun 26, 2014 at 2:15 AM, James A. Bednar notifications@github.com wrote:

Have you had a chance to try to fix the errors so that this code can be accepted? I'll be happy to merge it in once it works!

— Reply to this email directly or view it on GitHub https://github.com/ioam/imagen/pull/19#issuecomment-47174858.

Ramón Heberto Martínez Mayorquin

h-mayorquin commented 10 years ago

Hi, Jim.

Sorry for the long detail. Short story is that I got involved in a project over the summer and I could not get done with this. But now I have some time and I decided to take the time to finish this.

My first problem is that I fail to reproduce the error. I create a test that goes (for the value of the retinal density which is guess is x density for me) throughout a lot of values of grid density and it does not return errors. I attach the code here maybe you get an error with it and then I can produce to repair the error.

Otherwise maybe you can tell me what error are getting from Topographica. I did not use the package so I am not familiar with it and don't have it installed.

Test function



from imagen import random
import numpy as np
import matplotlib.pyplot as plt
from matplotlib import colors
from imagen.boundingregion import BoundingBox
import os

scale = 1
offset = 0
bound = BoundingBox(points=((-2,-2), (1,1)))
bound = BoundingBox(radius = 1)
seed = 206
xdensity = 48
ydensity = 48
size_of_cells = 1
grid_density = 1.0 / size_of_cells
grid_density = 1

grid_densities = np.arange(5, xdensity, 0.1)

for grid_density in grid_densities:

    print '******************'
    print '******************'
    print 'grid density=',  grid_density

    noise = random.DenseNoise(grid_density = grid_density, bounds = bound, xdensity = xdensity, ydensity = ydensity, 
                          scale = scale, offset = offset, random_generator=np.random.RandomState(seed=seed)) 

    cmap = colors.ListedColormap(['black', 'gray', 'white'])

    bounds = [offset - 0.25*scale, offset + 0.25*scale, offset + scale - 0.25*scale, offset + scale + 0.25*scale ]
    ticks = [offset - 0.25*scale, offset + 0.25*scale, offset + scale - 0.25*scale, offset + scale + 0.25*scale]
    norm = colors.BoundaryNorm(bounds, cmap.N)

    y = noise() # If you keep calling noise() it keeps producing different random numbers 

    img = plt.imshow(y, cmap=cmap, norm=norm, interpolation='nearest') # norm = colors.Normalize(vmin=-1,vmax=1)
    cbar = plt.colorbar(img, cmap=cmap, norm=norm, boundaries=bounds, ticks=ticks) # Ticks controls the values 
    cbar.ax.set_ylabel('Luminance')

    print "No error"  
    print '******************'

You can show the images with plt.show() if you want to see that they snap.

jbednar commented 10 years ago

Thanks for coming back to this!

Try changing bound = BoundingBox(radius = 1) to bound = BoundingBox(radius = 0.6) in your script, and then re-running it. Your script then gives me the error below right away. I get the same result for most other non-integer radii, e.g. 1.9, yet others like 2.1 give no problem. So it looks like the problem is dependent on both grid_density and the bounds.

Jim



grid density= 5.0 Traceback (most recent call last): File "./topographica", line 41, in process_argv(argv[1:]) File "/home/jbednar/research/topographica/topo/misc/commandline.py", line 708, in process_argv execfile(filename,main.dict) File "external/imagen/noise2.py", line 39, in y = noise() # If you keep calling noise() it keeps producing different random numbers File "/home/jbednar/research/topographica/external/imagen/imagen/random.py", line 60, in call result = self._distrib(shape,p) File "/home/jbednar/research/topographica/external/imagen/imagen/random.py", line 219, in _distrib A[j][i] = Z[outcome2][outcome1] IndexError: index 5 is out of bounds for axis 0 with size 5

jbednar commented 10 years ago

In any case, it's probably a good idea to test with the density, bounds, and grid_density chosen from random distributions once you address the above issue, since it's hard to think of all the possible combinations that could cause problems. Of course, problems are more likely to arise at edge cases, and random sampling might take a long time to discover those, so it's best to have both specific test cases and random ones.

h-mayorquin commented 9 years ago

Hi, James so I modify the classes and it seems that now they do pass the random test.

I attach both test (one for the Dense and for one for the Sparse now bellow)

Dense

from imagen import random
import numpy as np
import matplotlib.pyplot as plt
from matplotlib import colors
from imagen.boundingregion import BoundingBox
import os

scale = 1
offset = 0
bound = BoundingBox(points=((-2,-2), (1,1)))
bound = BoundingBox(radius = 1.1)
seed = 206
xdensity = 4
ydensity = 4
size_of_cells = 1 
grid_density = 1.0 / size_of_cells
grid_density = 2

N_test = 10000

min_density = 1
max_density = 10
max_radius = 2

for x in xrange(N_test):

    print '---------------------------------'
    print 'test number', x 

    xdensity = np.random.rand() * (max_density - min_density ) +  min_density
    ydensity = np.random.rand() * (max_density - min_density ) +  min_density
    #ydensity = xdensity 

    aux = 1.0 / np.max((xdensity, ydensity)) # Min pixel size 

    max_grid_density = np.min((xdensity, ydensity))

    grid_density = np.random.rand() * (max_grid_density - min_density)  + min_density

    aux2 = 1.0 / grid_density # Grid size 
    min_radius = np.max((aux, aux2)) # radius has to be greater than grid size and pixel size 

    radius = np.random.rand() * (max_radius - min_radius) + min_radius 

    bound = BoundingBox(radius = radius)

    print 'values'
    print 'xdensity', xdensity
    print 'ydensity',  ydensity
    print 'grid density', grid_density
    print 'radius', radius 

    noise = random.DenseNoise(grid_density = grid_density, bounds = bound, xdensity = xdensity, ydensity = ydensity, 
                          scale = scale, offset = offset, random_generator=np.random.RandomState(seed=seed)) 

    y = noise()

And for the Sparse Case:

from imagen import random
import numpy as np
import matplotlib.pyplot as plt
from matplotlib import colors
from imagen.boundingregion import BoundingBox
import os

scale = 1
offset = 0
bound = BoundingBox(points=((-2,-2), (1,1)))
bound = BoundingBox(radius = 1.1)
seed = 206
xdensity = 4
ydensity = 4
size_of_cells = 1 
grid_density = 1.0 / size_of_cells
grid_density = 2

N_test = 10000

min_density = 1
max_density = 10
max_radius = 2

for x in xrange(N_test):

    print '---------------------------------'
    print 'test number', x 

    xdensity = np.random.rand() * (max_density - min_density ) +  min_density
    ydensity = np.random.rand() * (max_density - min_density ) +  min_density
    #ydensity = xdensity 

    aux = 1.0 / np.max((xdensity, ydensity)) # Min pixel size 

    max_grid_density = np.min((xdensity, ydensity))

    grid_density = np.random.rand() * (max_grid_density - min_density)  + min_density

    aux2 = 1.0 / grid_density # Grid size 
    min_radius = np.max((aux, aux2)) # radius has to be greater than grid size and pixel size 

    radius = np.random.rand() * (max_radius - min_radius) + min_radius 

    bound = BoundingBox(radius = radius)

    print 'values'
    print 'xdensity', xdensity
    print 'ydensity',  ydensity
    print 'grid density', grid_density
    print 'radius', radius 

    noise = random.SparseNoise(grid_density = grid_density, bounds = bound, xdensity = xdensity, ydensity = ydensity, 
                          scale = scale, offset = offset, random_generator=np.random.RandomState(seed=seed)) 

    y = noise()
h-mayorquin commented 9 years ago

Hi, I was wondering if everything was OK with the code or maybe you have been just been very busy on the other side.

jbednar commented 9 years ago

Sorry -- I had been unable to test the code properly due to an unrelated bug, and managed to forget about this while waiting for someone to fix that bug. I've now fixed it, and the new version of your code seems to work fine. I cleaned that up as indicated previously (formatting, docstrings, parameter definitions, assertions, etc.), and I've now committed that to imagen (https://github.com/ioam/imagen/commit/c6d48fbe). Thanks for your contribution, which is now part of imagen for everyone!