april-tools / cirkit

a python framework to build, learn and reason about probabilistic circuits and tensor networks
https://cirkit-docs.readthedocs.io/en/latest/
GNU General Public License v3.0
71 stars 1 forks source link

introduce random context manager #58

Closed lkct closed 1 year ago

lkct commented 1 year ago

After discussion, we decide: The random_state introduced in #44 is reverted for now. We'll discuss whether to bring it back when the implementation of the library becomes more stable. (We will continue on this in #61)

Also add test on EiNet to guarantee the result is reproducible.

lkct commented 1 year ago

Better thread safety.

Problem is, can we guarantee other parts of the code is thread-safe? Seems to me an easier solution is to claim thread-safety is not guaranteed for now. (However, it's indeed better if we can fully manage it.)

With the presence of GIL, I don't think we can gain much from multithreading, and multiprocessing should be preferred instead.

More control by the user.

Do we need to support this? What if a user wants to use a torch random state instead? There's always a trade-off between letting users decide what to do and telling users what to do.

For code not involving torch

That's a more fundamental problem. Our code will be based more on torch. Also we should try to manage the source of randomness, preferably single-source it.

Here's a question: what's the suggested practice to construct a model based on a random binary tree with controlled randomness?

following scikit-learn that has an amazing API for this in my opinion.

sklearn is good. But as above, I would incline to the torch style. (In sklearn, the same practice is applied to the whole package, which makes it better)


To Be Discussed At The Next Meeting