🐞use fractions.Fraction to avoid rounding error in probabilities sum

[acarril]

fix #25

[acarril]

That sounds good to me! Fractions was only the first thing that came to mind.

…

On Sun, Oct 15, 2023 at 20:22 Manuel Martinez ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/stochatreat/stochatreat.py <#26 (comment)> : > + probs_sum = float(np.array([Fraction(f).limit_denominator() for f in probs]).sum()) + if probs_sum != 1: I would use math.isclose <https://github.com/manmartgarc/stochatreat/blob/main/> for checking this. The default relative tolerance is 1e-09 which might be sufficient for this. The main reason I'd use this as opposed to Fraction is that it's clearer, at least to me, what we are checking here. Another reason is that it avoids creating new Fraction objects and calling the limit_denominator method. This is definitely not relevant in practice since in reality users would supply one probability value per treatment arm and memory should never be an issue in this case. Happy to go with either after the formatting/linting issues are resolved. — Reply to this email directly, view it on GitHub <#26 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACKSB3FIPPRQXLGYM7BLS4LX7R42ZANCNFSM6AAAAAA6AMU5TA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[manmartgarc]

Would you like to keep working on this? I can't merge as it is because of the failing checks.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.04%. Comparing base (a8c6e14) to head (b7c753b).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #26      +/-   ##
==========================================
+ Coverage   93.90%   94.04%   +0.14%     
==========================================
  Files           3        3              
  Lines          82       84       +2     
==========================================
+ Hits           77       79       +2     
  Misses          5        5              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[manmartgarc]

@acarril better late than never, sorry for the delay - feel free to add your review :)

PS: I think we should probably add a check for the relative size of treatments and data; I shouldn't be able to pass more treatments than rows!

[acarril]

this looks good to me! thanks for implementing it.

regarding the check, it sounds about right, but probably a separate issue?