Fixed RandomUtils.NextFloat() extension methods #177
Conversation
Update fork...
Update local fork
Update fork
Update fork
Setting 'useHeader: false' in 'TrainAndPredictIrisModelTest' because …
Update fork
Update Fork
…d NextSingle() -> NextFloat()
| /// </summary> | ||
| Single NextSingle(); | ||
| Single NextFloat(); |
There was a problem hiding this comment.
NextFloat [](start = 15, length = 9)
Hi @zeahmed thanks for the change.
I understand that it results in fewer files changed, but let's please not change IRandom. I want to get rid of the RandomUtils, but I'm less enthusiastic about making IRandom resemble it. Maybe use VS to rename this back to NextSingle.
More generally: Float (capital F) is not a concept we want to expose. Back when we had "single" and "double" precision builds, with Float type aliases in practically every file it was a concept in this codebase, but those days are long over. Now then, the RandomUtils, being a shim to facilitate that, exposed this. (Once upon a time, there was an #ifdef surrounding that block of code you've now removed, plus a few alternate codepaths.) But we should be working on removing vestigates from that time rather than having them infect "clean" innocent interfaces that had nothing to do with that. :D #Closed
There was a problem hiding this comment.
Make sense. I will rename NextFloat() -> NextSingle().
Removed two NextFloat() extension methods from RandomUtils and replaced all usage of them with `IRandom.NextSingle()`.
ghost
locked as resolved and limited conversation to collaborators
Fixing #166
Removed two NextFloat() extension methods from RandomUtils and renamed NextSingle() to NextFloat()
Currently, NextFloat() is used everywhere instead of NextSingle().
Actually, for some implementation NextFloat() actually calls NextSingle() internally. Let me know if NextSingle() needs to be preserved.