-
Notifications
You must be signed in to change notification settings - Fork 2.3k
random_color()
and random_bright_color()
should have a parameter random_seed
#1671
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
Comments
The link you have posted regarding the numpy random seed doesn't really apply for the |
True, sorry! |
Hi, Can I take this issue? |
there was already a pull request one month ago here: #1676 |
@kolibril13 I reviewed PR #1676, it works but I don't think we need if-else code statement, we can do it directly also.
So should I create a new pr? |
Hey, I have reviewed that PR already a while ago, but the fundamental issue noted in #1676 (comment) has not been discussed or resolved yet. Removing the if-else will also not change that. |
I don't know if I understood it correctly. What is the expected behavior? Is it if I looked into it and I don't think its possible with python random module. Random does give us function such >>> import random
>>> random.getstate()[1][0]
2147483648
>>> random.seed(42)
>>> random.getstate()[1][0]
2147483648 Though this can be done in >>> import numpy as np
>>> np.random.get_state()[1][0]
2147483648
>>> np.random.seed(42)
>>> np.random.get_state()[1][0]
42
>>> np.random.randint(0, 100)
51
>>> np.random.randint(0, 100)
92
>>> np.random.randint(0, 100)
14
>>> np.random.get_state()[1][0]
723970371 But it also changes after some tries, so we can't determine the seed. I found something on stackoverflow related to it:
So, I don't think we can retrieve current Random seed as the seed is used to update the internal state of the random number generator is not directly stored anywhere. References: |
Before we get into implementation details, we have to agree on what kind of behavior we should actually expect. Personally, I think that a situation like >>> random_bright_color(42)
<Color #f5d5e0>
>>> random_bright_color(42)
<Color #f5d5e0> is not desireable, because setting the seed then is equivalent to hard-coding one specific color. I would expect that repeatedly calling You are right that the current seed of an initialized RNG cannot be retrieved -- but as it is mentioned in the SO answers you've posted, we can still store the seed of the RNG that has been initialized last somewhere. (Maybe in Manim's configuration, maybe "just" in the module; not sure about that.) Then, when |
Yeah, we can store seed in config file and compare it if users provide
I was thinking maybe we could use In the meanwhile, I can try working on storing it in config system. |
Has anyone closed this issue? Can I tackle it? @RaghavPrabhakar66 @behackl |
I've had a look through all of the stuff discussed in this issue so far and I'm not sure the solution of globally storing a seed for the random function is ideal because it opens the door for other areas of the code to reference that same seed and then produce inconsistent results if the code is edited. Consider the two following snippets of code: colour1 = random_color(42)
colour2 = random_color(42)
shape1 = random_shape(42)
shape2 = random_shape(42) colour1 = random_color(42)
shape1 = random_shape(42)
colour2 = random_color(42)
shape2 = random_shape(42) These two snippets, although only a trivial reordering of one another, would produce very different results (assuming that random_shape accesses the same seed/RNG) and I don't think it's immediately obvious that either I can think of a few solutions right now, each of course with their own benefits and drawbacks:
Comments on the above solutions:
Let me know what you think of the above and we can discuss further before I actuallly write any code. Feel free to @ me (Phosphorescent#2786) in the Discord if you'd rather talk there :) |
Hi! I noticed that this issue has been open for a while. I'm looking to make my first contribution, so I don't have a lot of experience with the project, but I agree that creating a separate function seems like the best idea. Introducing non-deterministic behavior like that seems like a nightmare. Is anyone else still actively working on this issue, or can I go ahead and start implementing that approach? |
just an additional comment: I'd suggest a parameter to exclude either certain colors or at least to optionally automatically exclude the current background color. |
That's a good point. I imagine re-rendering something until you get an actually visible color gets more difficult as you work with more colors. Actually, wouldn't the function have to exclude a range of very similar colors to avoid that situation? |
I don't think so, since it anyway are not too many different colors from which the random colors are drawn - but It would probably not too difficult to find similar colors to exclude using hue and brightness. |
I think that goes beyond the scope of this issue, but it does sound like enough of a concern to deserve its own issue. I'm not too sure though. What do you think? |
I am just thinking that it would be easiest to implement these things at the same time rather than doing surgery on this code several times in a row. And I also think that just excluding the current background is "good enough" - I reverted to |
Sounds good. I'm not experienced enough with Manim to really know how dificult/easy using these functions are |
Would you like to clarify what you mean by excluding "background" ? As per the color PR it uses the manim core colors for random colors. Or like the basic manim colors |
Nvm. So the current goal of this issue becomes changing the behavior with the seed but I also think that the exclusion of a specific color range shouldn't be done in this. Because in the end it gets more complicated if you want to have a magic function which basically computes legible colors if you specify a background i actually think that one should be separate if we want to do it right we need to do some research what that actually means. |
I have a suggestion here - I created a small class RandomColor(object):
def __init__(self, seed = None) -> None:
self.generator = np.random.RandomState(seed=seed)
self.palette = ["#FFFFFF"]
def setPalette(self, palette : Sequence[ParsableManimColor]):
self.palette = palette
def randomPaletteColor(self):
return self.generator.choice(self.palette)
def randomHue(self, reference_color:ParsableManimColor = "#FF0000", saturation=None, lightness=None, value=None):
if saturation != None:
if lightness != None:
rgb = colorsys.hls_to_rgb(h=self.generator.uniform(0,1),s=saturation,l=lightness)
elif value != None:
rgb = colorsys.hsv_to_rgb(h=self.generator.uniform(0,1),s=saturation,v=value)
else:
rgb = colorsys.hsv_to_rgb(h=self.generator.uniform(0,1),s=saturation,v=1)
else:
hsv = colorsys.rgb_to_hsv(*color_to_rgb(reference_color))
rgb = colorsys.hsv_to_rgb(h=self.generator.uniform(0,1),s=hsv[1],v=hsv[2])
return ManimColor(rgb) usage: class randTest(Scene):
def construct(self):
rndcol = RandomColor()
rndcol.setPalette([RED,GREEN,BLUE,YELLOW])
for i in range(len(rndcol.palette)):
self.add(Rectangle(width=1, height=0.3, fill_opacity=1, color=rndcol.palette[i]).move_to([-6+i,3.5,0]))
self.add(Square(side_length=0.6, fill_opacity=1, color=PURE_BLUE).move_to(6.6*LEFT+0*UP))
self.add(Square(side_length=0.6, fill_opacity=1, color=RED).move_to(6.6*LEFT+1*DOWN))
self.add(Square(side_length=0.6, fill_opacity=1, color="#4c855b").move_to(6.6*LEFT+2*DOWN))
self.add(Square(side_length=0.6, fill_opacity=1, color="#e0d9c3").move_to(6.6*LEFT+3*DOWN))
for x in np.linspace(-6,6.5,20):
self.add(Dot(x*RIGHT+3*UP, radius=.3, color=rndcol.randomPaletteColor()))
self.add(Dot(x*RIGHT+0*UP, radius=.3, color=rndcol.randomHue(PURE_BLUE)))
self.add(Dot(x*RIGHT+1*DOWN, radius=.3, color=rndcol.randomHue(RED)))
self.add(Dot(x*RIGHT+2*DOWN, radius=.3, color=rndcol.randomHue("#4c855b")))
self.add(Dot(x*RIGHT+3*DOWN, radius=.3, color=rndcol.randomHue("#e0d9c3"))) |
Ok I continued to work on it, ready to suggest a pull-request, after some additional polishing. What does everyone/anyone think?
class randTest(Scene):
def construct(self):
rndcol = RandomColor(seed=42)
rndcol.setPalette([RED,GREEN,BLUE,YELLOW])
for i in range(len(rndcol.palette)):
self.add(Rectangle(width=1, height=0.3, fill_opacity=1, color=rndcol.palette[i]).move_to([-4+i,3.5,0]))
self.add(Tex(r"randomPaletteColor()", font_size=22).move_to([-7,3,0],aligned_edge=LEFT))
self.add(Tex(r"randomColor()", font_size=22).move_to([-7,2.3,0],aligned_edge=LEFT))
self.add(Rectangle(height=0.6, width=14, fill_opacity=1, color="#e0d9c3").move_to(7*LEFT+1.5*UP, aligned_edge=LEFT))
self.add(Tex(r"randomContrastColor()", font_size=22,color=BLACK).move_to([-7,1.5,0],aligned_edge=LEFT))
self.add(Rectangle(height=0.6, width=14, fill_opacity=1, color=PURE_BLUE).move_to(7*LEFT+0.5*UP, aligned_edge=LEFT))
self.add(Tex(r"randomContrastColor()", font_size=22,color="#768040").move_to([-7,0.5,0],aligned_edge=LEFT))
self.add(Square(side_length=0.6, fill_opacity=1, color="#FFFF00").move_to(5.5*LEFT+0.5*DOWN))
self.add(Square(side_length=0.6, fill_opacity=1, color=RED).move_to(5.5*LEFT+1.5*DOWN))
self.add(Square(side_length=0.6, fill_opacity=1, color="#4c855b").move_to(5.5*LEFT+2.5*DOWN))
self.add(Square(side_length=0.6, fill_opacity=1, color="#e0d9c3").move_to(5.5*LEFT+3.5*DOWN))
self.add(Tex(r"randomHue()", font_size=30,color="WHITE").rotate(90*DEGREES).move_to([-6.5,-2,0],aligned_edge=LEFT))
for x in np.linspace(-4.5,6.5,15):
self.add(Dot(x*RIGHT+3*UP, radius=.3, color=rndcol.randomPaletteColor()))
self.add(Dot(x*RIGHT+2.3*UP, radius=.3, color=rndcol.randomColor()))
self.add(Dot(x*RIGHT+1.5*UP, radius=.25, color=rndcol.randomContrastColor("#e0d9c3")))
self.add(Dot(x*RIGHT+0.5*UP, radius=.25, color=rndcol.randomContrastColor(PURE_BLUE)))
self.add(Dot(x*RIGHT+0.5*DOWN, radius=.3, color=rndcol.randomHue("#FFFF00")))
self.add(Dot(x*RIGHT+1.5*DOWN, radius=.3, color=rndcol.randomHue(RED)))
self.add(Dot(x*RIGHT+2.5*DOWN, radius=.3, color=rndcol.randomHue("#4c855b")))
self.add(Dot(x*RIGHT+3.5*DOWN, radius=.3, color=rndcol.randomHue("#e0d9c3"))) |
Uh oh!
There was an error while loading. Please reload this page.
As it is always good to reproduce randomness, these two functions should have a parameter seed.
https://docs.manim.community/en/stable/_modules/manim/utils/color.html#random_color
Then it should be possible to just init that seed.
https://towardsdatascience.com/stop-using-numpy-random-seed-581a9972805fEDIT: Second link does not apply here.
The text was updated successfully, but these errors were encountered: