nvzqz/RandomKit

Possibly Revert #26 and Related Changes

nvzqz opened this issue · 2 comments

nvzqz commented

I'd like to weigh the pros and cons of keeping the changes made by #26.

Some of the cons I can think of:

  • Greatly increased compile times
  • Introduced methods to types that are possibly outside of the scope of this library
    • exponentOfE()
    • raised(to:)
    • squareRoot()
    • log()
  • The distributions aren't very well documented. Their use cases aren't immediately obvious unless one goes through the related Wikipedia articles.
  • Most developers using RandomKit will likely not need these features

An alternative solution I would like to propose is another library that has a dependency on RandomKit and features this.

This would be a breaking change, causing the version number for RandomKit to be bumped up to 3, which I'm ok with doing. If this change is made, other helpful breaking changes should also be made.

I would like to know what @phimage's opinion is on this, since he made the changes.

phimage commented

It's true that many developers use only uniform distribution and think mistakenly that random means random uniform.
That's why I make this PR. I think about RandomKit not as a RandomUniformKit only.

Random distribution is more used in scientist/mathematician and economist worlds (see r language,stata,sas,python[numphy]) to forecast etc...

I am not afraid of compilation times. It's a little framework. The tools are well made to not compile it every day... Execution times is more important.

Yes! cut into module it's always a good practice.
(and I agree that fake name, and others stuffs must be out (#12))
So I agree to cut into an other project, RandomDistributionKit.

The con is only maintenance when one is updated, the other one could not compile.
If everyone use Cocoapod, a subspec will be sufficient but that's not the case. git independent project is more suited for carthage and spm

nvzqz commented

With regard to random distribution, I strongly agree that it should be placed into its own project/module. I'll be removing this from RandomKit, which is a breaking change requiring a version 3.