Incorrect value threshold of minimum radius size 100 (same as threshold for max value) leading to error in numpy randint

[Schinzilord]

Thanks for providing the code! Upon playing around with the parameters, I detected the following bug:

The following minimum example:

parameters = {'width': 700, 'height': 700, 'polygon_size': 150, 'color_bleed': 3,
              'colors': ['#000000', '#FFFFFF'],
             'spots': {'amount': 500, 'radius': {'min': 100, 'max': 200}, 'sampling_variation': 20},
             'pixelize': {'percentage': 1, 'sampling_variation': 20, 'density': {'x': 70, 'y': 50}}}           
image = camogen.generate(parameters)

leads to the following error messages:

Traceback (most recent call last):
  File "./gen_camo.py", line 39, in <module>
    image = camogen.generate(parameters)
  File "/home/schinzilord/Dokumente/Programmierung/Python/camogen/generate.py", line 176, in generate
    image = generate_image(pattern)
  File "/home/schinzilord/Dokumente/Programmierung/Python/camogen/generate.py", line 138, in generate_image
    add_spots(pattern, image, draw)
  File "/home/schinzilord/Dokumente/Programmierung/Python/camogen/helpers.py", line 274, in add_spots
    spot_radius = np.random.randint(pattern.spots_radius_min, pattern.spots_radius_max)/2
  File "mtrand.pyx", line 746, in numpy.random.mtrand.RandomState.randint
  File "_bounded_integers.pyx", line 1254, in numpy.random._bounded_integers._rand_int64
ValueError: low >= high

Root Cause if the following limit setting in def init of Class pattern:

# Spots
            if 'spots' in parameters.keys():
                self.spots_nbr = min(20000, np.abs(parameters['spots']['amount']))
                self.spots_radius_min = max(5, min(100, np.abs(parameters['spots']['radius']['min'])))
                self.spots_radius_max = max(5, min(100, np.abs(parameters['spots']['radius']['max'])))
                self.spots_sampling = np.abs(parameters['spots']['sampling_variation'])
            else:
                self.spots_nbr = 0

Maximum values both for min radius and max radius is 100, leading to an error later on in def add_sports in line 274: spot_radius = np.random.randint(pattern.spots_radius_min, pattern.spots_radius_max)/2 Numpy integer random number generator is expecting min values < max value.

A possible solution could be to set minimum value to max value minus sampling size or min = max - 1. Best, Stefan

[glederrey]

Dear Stefan,

I'm always amazed to see people still play with this library. =) I could almost add it to pypi. =)

From what I see, I am not allowing a spot radius outside of the range [5, 100]. (I can't remember why though) Since you set both values to be 100 (for the min) and 200 (for the max), these two values are "corrected" to be 100 and 100. So, since these two values are equal, the function randint is throwing an exception.

Honestly, I do not know why I'm blocking the radius values to be bigger than 100. I think the easiest would simply be to remove the upper bound and set the lower bound to 0. (even though it won't create any spots). What do you think about this solution?

Best, Gael

[glederrey]

I just had a quick look at the code. Many parameters are blocked between two bounds. I think it would make sense to simply remove these bounds. It might not lead to "nice" images. But it would allow for more creativity.

If you agree with this solution, I will update the repo in the following days.

[Schinzilord]

Dear Gael,

thanks for coming back so quickly. I agree, it would improve creatity to drop any boundaries. However, I suggest then to throw meaningful error messages, e.g. in helper.py before line 274:

if pattern.spots_radius_min >= pattern.spots_radius_max:
            raise ValueError('Minimum radius needs to be smaller than maximum spot radius')

Another potential problem could exist, if you drop the boundary for "amount", and then somebody uses 1000000 which leads to a crash in the program or it runs forever... But since the code will not be used in a productive system out of the box, freedom is more important than stability :)

Best, Stefan

[glederrey]

Hi Stefan,

That's a good idea. I can definitely do it. However, I think it would be fair towards you to let you have this contribution since you showed me where's the problem and how to fix it. So, I think it would be good if you can fork the repo, make the fixes and then submit a PR. That way, you'll appear as a contributor.

If you plan to improve this project further, I can also directly add you as a collaborator. (I am not really working on it since I just used it for my master thesis a long time ago)

If you don't have time or you don't want to submit the PR, I will take care of it. But I think it's fair to let people be listed as contributors when they do something good for a project. =)

Best, Gael

[Schinzilord]

Thanks! I create a pull request https://github.com/glederrey/camogen/pull/5 for removing boundaries and improving error handling. Currently I have no plans of improving this project further, but maybe in the future :) Please review the pull request, I am of course happy to adjust and improve if necessary.

Best, Stefan

[glederrey]

Thanks a lot mate. I'll take a look at it now. =)

[glederrey]

Issue closed since PR#5 was accepted. Thanks again for helping. =)

[Schinzilord]

Thanks for the good cooperation and fast respone!