ihmeuw / vivarium_census_prl_synth_pop

US Census Probabilistic Record Linkage synthetic population generation
BSD 3-Clause "New" or "Revised" License
2 stars 1 forks source link

Random integers might include values one-beyond top of range #61

Closed aflaxman closed 2 months ago

aflaxman commented 2 years ago

https://github.com/ihmeuw/vivarium_census_prl_synth_pop/blob/522f920a35202a98b9c3afa5f99fc91ff623162b/src/vivarium_census_prl_synth_pop/utilities.py#L256

I'm looking for weirdness in the SSN generator, and I don't think this is it. But since you are rounding here, I think you can end up with numbers that include the upper limit, and I think you intend it to be exclusive. Floor would fix, if I'm not mistaken about this.

aflaxman commented 2 years ago

Oh, but looking more at how this is used, perhaps you want it to include the upper bound, so maybe the solution is a change to the docstring.