Skip to content

path randomness is repetitive (sketch RNG seeded to 0 for each path anew) #13479

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

Open
eudoxos opened this issue Feb 21, 2019 · 17 comments · May be fixed by #26050
Open

path randomness is repetitive (sketch RNG seeded to 0 for each path anew) #13479

eudoxos opened this issue Feb 21, 2019 · 17 comments · May be fixed by #26050
Labels
keep Items to be ignored by the “Stale” Github Action

Comments

@eudoxos
Copy link

eudoxos commented Feb 21, 2019

Bug report

Sketch transformation has identical pseudo-random output for identical inputs

The sketch transformation (such as used in xkcd) produces exactly the same randomized result if the same path is plotted repeatedly. This is visible especially in the legend, where the rectangle patch is repeated for each item. I exaggerated randomness and scale to make the effect better visible.

As far as I can see, the culprit is in the built-in RNG which gets re-seeded to 0 for each Sketch instantionation:

.

import matplotlib.pyplot as plt
with plt.xkcd(randomness=40,scale=1):
    import matplotlib
    from matplotlib import gridspec
    pat,txt=plt.pie([10,20,30,40],wedgeprops={'edgecolor':'black'})
    plt.legend(pat,['first','second','third','fourth'],loc='best')

Actual outcome
image

zoom on the legend:

image

Expected outcome

Shapes are distored randomly, e.g. each rectangle in the legend is different.

Matplotlib version

  • Operating system: Ubuntu 18.04
  • Matplotlib version: 2.2.2
  • Matplotlib backend (print(matplotlib.get_backend())): TkAgg
  • Python version: 3.6.7
  • Jupyter version (if applicable): [none]
  • Other libraries:

installation: default packaged matplotlib installation (python3-matplotlib).

@eudoxos eudoxos changed the title sketch RNG is seeded the same for each path sketch RNG is seeded to 0 for each path, making path randomness repetitive Feb 21, 2019
@eudoxos eudoxos changed the title sketch RNG is seeded to 0 for each path, making path randomness repetitive path randomness is repetitive (sketch RNG seeded to 0 for each path anew) Feb 21, 2019
@WeatherGod
Copy link
Member

WeatherGod commented Feb 21, 2019 via email

@tacaswell tacaswell modified the milestones: v3.1.0, v3.2.0 Feb 21, 2019
@timhoffm
Copy link
Member

timhoffm commented Feb 21, 2019

Ideally, we would seed once per figure. That would make figures reproducible, but repeated elements within the figure random. Didn‘t check the code if that‘s easily possible.

@ImportanceOfBeingErnest
Copy link
Member

Note that if you seed once per figure the shape of a patch would depend on how many patches you have drawn beforehands.

@timhoffm
Copy link
Member

Yes, but if I create the same figure with the same code twice, it's the same.

@jklymak
Copy link
Member

jklymak commented Feb 21, 2019

seeding once would be good, and grabbing the python seed would be even better if that is at all possible.

@eudoxos
Copy link
Author

eudoxos commented Feb 22, 2019

Would it be acceptable to have the seed as a static variable inside the Sketch class, increment it every time its ctor is called (so that it is seeded with 0 for the first instance, 1 for the next one etc), plus somehow expose a python function to reset the seed to 0, which would be called from Figure ctor?

@ImportanceOfBeingErnest
Copy link
Member

As pointed out above, I would not consider this acceptable. There needs to be a way to ensure that a shape is the same, independent of how many other shapes there are drawn before in that figure.

@eudoxos
Copy link
Author

eudoxos commented Feb 22, 2019

As pointed out above, I would not consider this acceptable. There needs to be a way to ensure that a shape is the same, independent of how many other shapes there are drawn before in that figure.

If you look at the legend which I posted above, it is exactly this shape is the same which makes it so ugly and computerish, that's the subject of the issue.

If there is no general agreement, there must be a setting (in rcParams perhaps?) for that. The options are (not all necessarily to implemented, I am just writing out what is theoretically possible):

  1. seed with a constant: same shape looks the same, current situation; or
  2. seed with a changing seed, resetting to some intial value for each figure: same shape looks different in one figure, but two identical consecutive figures will look the same; or
  3. seed with a changing seed, never resetting (but starting with some given value when matplotlib is imported): nothing will look the same, but running Python more times will always give the same result; or
  4. seed with intiially random (e.g. time since epoch or such) and then changing seed: nothing every the same.

What I reported (as an issue) is 1. To me at least, 2. is a compromise between per-figure reproducibility (e.g. for testing) and random appearance.

@ImportanceOfBeingErnest
Copy link
Member

The only option I see is to let _sketch take an additional value seed in

def set_sketch_params(self, scale=None, length=None, randomness=None):

that will need to be (converted to) a valid seed before passing onto the C level. The C class needs to take this further argument.
With this being implemented you can already loop though all your artists and set some random or defined seed. Of course any additional features can be built on top, like per-figure-seed or whatever you like. Also for the xkcd style one can sure discuss which of those would make people most happy.

@jklymak
Copy link
Member

jklymak commented Feb 22, 2019

I think @ImportanceOfBeingErnest has the correct solution. Then unti tests can set the seed in python, as they already do many times in the tests.

@eudoxos
Copy link
Author

eudoxos commented Feb 25, 2019

Based on @ImportanceOfBeingErnest suggestion, I added passing the seed to related functions in this branch: https://github.com/matplotlib/matplotlib/compare/master...eudoxos:sketch-seed?expand=1 (WIP). get_sketch_params can optionally take only 3-tuple (without seed) so that older rc files are readable. Unless overridden by hand for some artist, the seed will still be always zero, just like it is now.

For the next thing, how to make the seed change for each next instance of the PRNG? My idea is to use a special value -1, meaning: "use the previous seed (stored in a static variable somewhere) plus one". Along with a new function which would set the "previous seed" to some fixed value, called automatically for each new Figure about to be rendered.

It would look like following:

  1. there is a static variable previous_seed_value;
  2. when a figure is about to be rendered, set previous_seed_value to rcParams.sketch.seed (0 by default, if rcParams.sketch is not specified); this can include the special value of -1;
  3. when the PRNG is instantiated, which happens for each artist (as I understand), which stores its own sketch params:
    • if the artist's seed is -1, set the intiial PRNG seed to the previous_seed_value+1;
    • else set the initial PRNG seed to that value;
    • save the initial PRNG seed value to the previous_seed_value.

Any comments on that?

@eudoxos
Copy link
Author

eudoxos commented Feb 25, 2019

This is what I am getting now, using the special seed value of -1, which means: use the last one and increment. There were a few gotchas but it seems to be working now. A question now is whether we should treat all seed numbers as the seed to use (which is all numbers right now, besides -1), or on the contrary, have all numbers as a beginning of evolving (increasing) seed series, and only one (0, probably) to mean the special constant seed (for testing).

Resetting to the beginning for each figure not yet done.

image

@timhoffm
Copy link
Member

timhoffm commented Sep 7, 2019

@eudoxos Are you interested to open a PR for adding the seed=0 parameter?

For the automatic change of the seed, I would need to think more about it.

@timhoffm timhoffm modified the milestones: v3.2.0, v3.3.0 Sep 7, 2019
@eudoxos
Copy link
Author

eudoxos commented Sep 9, 2019

@timhoffm just put return 0; right at the start of get_prng_seed : https://github.com/matplotlib/matplotlib/compare/master...eudoxos:sketch-seed?expand=1#diff-96ebd28dd61fbcd3787dc7f18b3b9109R928 does that work for you?

@QuLogic QuLogic modified the milestones: v3.3.0, v3.4.0 May 7, 2020
@QuLogic QuLogic modified the milestones: v3.4.0, v3.5.0 Jan 27, 2021
@QuLogic QuLogic modified the milestones: v3.5.0, v3.6.0 Sep 25, 2021
@QuLogic QuLogic modified the milestones: v3.6.0, v3.7.0 Jul 8, 2022
@QuLogic QuLogic removed this from the v3.7.0 milestone Jan 26, 2023
@story645
Copy link
Member

@eudoxos sorry you never got a response, any chance you're still interested in adding a seed parameter?

@eudoxos
Copy link
Author

eudoxos commented Apr 21, 2023

Hi, check out the branch https://github.com/eudoxos/matplotlib/tree/sketch-seed . I don't have resources to re-apply to the current matplotlib (though I think it could be quite straightforward), test, document etc to push it all way to merging. Please feel welcome to build upon what I did.

@oscargus oscargus mentioned this issue May 1, 2023
5 tasks
@AryanSheka AryanSheka linked a pull request Jun 3, 2023 that will close this issue
5 tasks
Copy link

This issue has been marked "inactive" because it has been 365 days since the last comment. If this issue is still present in recent Matplotlib releases, or the feature request is still wanted, please leave a comment and this label will be removed. If there are no updates in another 30 days, this issue will be automatically closed, but you are free to re-open or create a new issue if needed. We value issue reports, and this procedure is meant to help us resurface and prioritize issues that have not been addressed yet, not make them disappear. Thanks for your help!

@github-actions github-actions bot added the status: inactive Marked by the “Stale” Github Action label Apr 18, 2025
@story645 story645 added keep Items to be ignored by the “Stale” Github Action and removed status: inactive Marked by the “Stale” Github Action labels Apr 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep Items to be ignored by the “Stale” Github Action
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants