markovify icon indicating copy to clipboard operation
markovify copied to clipboard

Optional PR

Open asigalov61 opened this issue 5 years ago • 9 comments

I think replacing regular random with secrets.random should improve the generating pattern/uniqueness of the pattern.

These are just my humble suggestions and it is totally up to you guys... :)

asigalov61 avatar Jan 27 '21 23:01 asigalov61

Coverage Status

Coverage remained the same at 100.0% when pulling bcf42a36944af53aa11900e4c9f51a103ec1c87d on asigalov61:master into c8eb4cfcdbe48456e092b266f13a2a3ee41f4435 on jsvine:master.

coveralls avatar Jan 28 '21 02:01 coveralls

@jsvine Per your suggestion I added my markovify Piano to the list of implementations/examples.

Thanks.

asigalov61 avatar Jan 30 '21 08:01 asigalov61

Thanks for the suggestion, @asigalov61. Could you explain more about (a) the motivation for using secrets, and (b) the particular implementation here?

As far as I can tell, the standard random module seems like a good fit for this library. Per the Python documentation:

In particular, secrets should be used in preference to the default pseudo-random number generator in the random module, which is designed for modelling and simulation, not security or cryptography.

... but markovify lands pretty directly in the "modelling and simulation" realm. What benefit would secrets provide here, and what sort of performance increase/decrease would it cause?

And on your piano example: Really fun, thanks! Could you, however, submit that as a separate PR?

jsvine avatar Feb 04 '21 03:02 jsvine

@jsvine Thank you for complimenting my work. It means a lot to me. I will submit a separate PR with the link. Thank you.

As far as the secrets module goes... while the standard random module is decent and fits fine with your implementation, it is always a good idea to use a more random and more secure version of the random generator. Not only it is a best practice but it would also help to reduce Markov plagiarism and repetitions as it can do it under certain circumstances.

In general, using the secrets random should produce better results (both for text and music from my direct experience). I modified my copy with secrets.random just like my PR and it works slightly better IMHO.

Otherwise, you are correct, and the standard random module is probably fine to use too. Although if I were you I would advise the users about it and let them know about the plagiarism/repetitions possibilities.

As a note, in AI music, plagiarism and repetitions are a huge issue and this is where secrets.random helps, so I make my suggestion from this particular perspective. So using secrets may not be critical/needed for other purposes.

I hope this makes sense.

Thanks.

asigalov61 avatar Feb 16 '21 20:02 asigalov61

I will submit a separate PR with the link.

Great, thanks!

As far as the secrets module goes... while the standard random module is decent and fits fine with your implementation, it is always a good idea to use a more random and more secure version of the random generator. Not only it is a best practice but it would also help to reduce Markov plagiarism and repetitions as it can do it under certain circumstances.

As I understand it, random vs. secrets should have no effect on randomness at a human-perceptual level, nor on Markov repetitions. As the secrets module states, random is "designed for modelling and simulation," which is Markovify's use-case. As I understand it, what secrets provides is enhanced security (e.g., by making it much harder to guess outputs) — something not needed by Markovify.

That said, I'm open to being convinced otherwise. If you or someone else can point me to documentation that demonstrates secrets's superiority for simulation, that'd be great. Alternatively, perhaps you could provide a small script that demonstrates a difference in repetitions.

jsvine avatar Feb 19 '21 15:02 jsvine

@jsvine Apologies for the delayed response. I sorta missed your last response...

Yes, I agree that difference between secrets and standard random is not that big perceptually so I am not saying that it will offer a significant improvement.

However, from my understanding, secrets.random uses crypto algos + oher tricks to not only make it as random as possible, but also it provides a much better random distribution, which is quite similar to how AI models do it, which is why I suggested looking into it.

I will try to make an example for you if I can to show how and where it is different, but please give me some time as it is not a priority issue for both of us.

And thank you very much again for hearing my suggestions and for making this awesome markov-chain implementation. Much much appreciated.

Alex.

PS. I was even able to generate multi-instrumental music with markovify. Check it out here if you are curious. Thanks. https://github.com/asigalov61/Markovify-Piano/tree/main/Output_Samples/Multi-Instrumental

asigalov61 avatar Mar 16 '21 15:03 asigalov61

@jsvine Btw, I wanted to kindly ask you to add my Markovify Piano to your list, please because for the love of God I can't figure out how to make a separate pull request for README. I still can't figure out GitHub PRs so your help will be much appreciated.

Or, if you can tell me how to do it, it would be great too but I think it is just easier to add manually IMHO.

Thank you.

Here is the code:

[Markovify Piano](https://github.com/asigalov61/Markovify-Piano) Coherent and plausible music generation with Markov-chain/model.

asigalov61 avatar Mar 16 '21 15:03 asigalov61

Btw, I wanted to kindly ask you to add my Markovify Piano to your list

No problem, now done: https://github.com/jsvine/markovify/commit/b0bfd2ab88d36680e358f17073a6378855fc7bdd

jsvine avatar Mar 26 '21 22:03 jsvine

@jsvine Thank you very much for your help and for the great software. If you need any help or want to share something, do not hesitate to contact me. Would be happy to help in return.

Alex

asigalov61 avatar Mar 28 '21 19:03 asigalov61