I'm creating this thread to discuss the change suggested in this PR 
<https://github.com/sympy/sympy/pull/28561>. This PR does fix a serious 
bug, at least partially which wouldn't allow us to sample discrete finite 
RVs (even the Coin RV of SymPy with default constructor parameters!). The 
details of the fix are highlighted clearly in the PR description, but we 
can discuss it further over here. I believe this implementation is superior 
to the current implementation because it respects the object types of the 
sample space and doesn't force them to be Python native types int and 
float. This is particularly useful if you need to preserve precision or use 
symbolic computation in downstream processing (e.g. not being forced to 
convert rationals or irrationals in the sample space to integers for 
sampling). 

However, this leads to a test failing 
<https://github.com/sympy/sympy/blob/ce3180c6cbb6b78dbb5b8a0bfa1402ade7d8d398/sympy/stats/tests/test_rv.py#L211>
 (sympy/stats/tests/test_rv.py:211). 
The test fails because it does a naive isinstance(sample(X+Y), float) check 
on the sampling output of a random variable defined as the *sum of a 
standard normal and a Die RV*. As discussed above, the output, if 
implemented correctly of sampling a Die RV should be a SymPy Integer, thus 
the type of sample(X+Y) will be a SymPy float. Since SymPy float is not a 
derived class of Python's native float, this test fails. 

As a solution, I propose increasing the scope of this test to check for 
*any* valid floating point type or if we're being pedantic, the SymPy float 
type. We cannot have a correct implementation of sample for all the test 
cases mentioned in the PR (and several more, complicated ones as well!) and 
this test case. 

Would love to know the maintainers' thoughts on the same. 

P.S. The PR is an incomplete draft PR for now. Further commits handling 
cases when the library is not specified as SciPy and regression tests etc 
will be added soon after I get some feedback or discussion on the 
approach.  

-- 
You received this message because you are subscribed to the Google Groups 
"sympy" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion visit 
https://groups.google.com/d/msgid/sympy/7326cf04-55df-4f48-9a9e-9e49f41de75cn%40googlegroups.com.

Reply via email to