kscience/kmath
9
0
Fork 1
Code Issues Pull Requests 6 Packages Projects Releases 16 Wiki Activity

Implement Commons RNG-like samplers in kmath-prob module in common source set #96

Closed
CommanderTvis wants to merge 0 commits from dev into dev
pull from: dev
merge into: kscience:dev
Conversation 2 Commits - Files Changed -
CommanderTvis commented 2020-06-07 18:14:53 +03:00 (Migrated from github.com)
Copy Link

closes #62

closes #62
altavir (Migrated from github.com) requested changes 2020-06-07 20:29:53 +03:00
altavir (Migrated from github.com) left a comment
Copy Link

It does not make sense to completely duplicate commons API and wrap it inside our API. If we are doing a new implementation, it should adhere to our API from the beginning. The API is of course open for debate.

It does not make sense to completely duplicate commons API and wrap it inside our API. If we are doing a new implementation, it should adhere to our API from the beginning. The API is of course open for debate.
kmath-commons-rng-part/src/commonMain/kotlin/scientifik/commons/rng/UniformRandomProvider.kt
altavir (Migrated from github.com) commented 2020-06-07 20:15:21 +03:00
Copy Link

We already have RandomGenerator. This one should be replaced

We already have RandomGenerator. This one should be replaced
kmath-commons-rng-part/src/commonMain/kotlin/scientifik/commons/rng/sampling/SharedStateSampler.kt
altavir (Migrated from github.com) commented 2020-06-07 20:17:37 +03:00
Copy Link

This should be removed. Current ideology is to set a generator once during chain creation. If we want to change the API, we need to discuss it. We can still use state for forks.

This should be removed. Current ideology is to set a generator once during chain creation. If we want to change the API, we need to discuss it. We can still use state for forks.
kmath-commons-rng-part/src/commonMain/kotlin/scientifik/commons/rng/sampling/distribution/AhrensDieterExponentialSampler.kt
altavir (Migrated from github.com) commented 2020-06-07 20:19:07 +03:00
Copy Link

Should be converted to Distribution

Should be converted to `Distribution`
altavir (Migrated from github.com) commented 2020-06-07 20:20:19 +03:00
Copy Link

Should be replaced by kotlinistic initializer. Maybe lazy one.

Should be replaced by kotlinistic initializer. Maybe lazy one.
kmath-commons-rng-part/src/commonMain/kotlin/scientifik/commons/rng/sampling/distribution/BoxMullerNormalizedGaussianSampler.kt
altavir (Migrated from github.com) commented 2020-06-07 20:22:13 +03:00
Copy Link

Should add references for commons-math classes everywhere for a proper tribute.

Should add references for commons-math classes everywhere for a proper tribute.
kmath-commons-rng-part/src/commonMain/kotlin/scientifik/commons/rng/sampling/distribution/ContinuousSampler.kt
altavir (Migrated from github.com) commented 2020-06-07 20:22:35 +03:00
Copy Link

Remove and replace by Double sampler

Remove and replace by Double sampler
kmath-commons-rng-part/src/commonMain/kotlin/scientifik/commons/rng/sampling/distribution/DiscreteSampler.kt
altavir (Migrated from github.com) commented 2020-06-07 20:23:01 +03:00
Copy Link

To be replaced Int sampler

To be replaced Int sampler
kmath-commons-rng-part/src/commonMain/kotlin/scientifik/commons/rng/sampling/distribution/InternalUtils.kt
altavir (Migrated from github.com) commented 2020-06-07 20:24:51 +03:00
Copy Link

Init blocks should remover wherever possible and replaced by factory methods.

Init blocks should remover wherever possible and replaced by factory methods.
kmath-commons-rng-part/src/commonMain/kotlin/scientifik/commons/rng/sampling/distribution/LargenMeanPoissonSampler.kt
altavir (Migrated from github.com) commented 2020-06-07 20:26:15 +03:00
Copy Link

Probably should be refactored to use companion functions instead of secondary constructors.

Probably should be refactored to use companion functions instead of secondary constructors.
kmath-prob/build.gradle.kts
@ -0,0 +2,4 @@
id("scientifik.mpp")
}
kotlin.sourceSets {
altavir (Migrated from github.com) commented 2020-06-07 20:27:34 +03:00
Copy Link

Should be done inside kmath-prob, not in a separate module.

Should be done inside kmath-prob, not in a separate module.
Shimuuar commented 2020-06-07 20:34:56 +03:00 (Migrated from github.com)
Copy Link

I only took a cursory look at PR, and I don't quite understand what is going on. I can only say that class should have some documentation explaining what they're doing.

I only took a cursory look at PR, and I don't quite understand what is going on. I can only say that class should have some documentation explaining _what_ they're doing.
CommanderTvis commented 2020-06-23 18:21:42 +03:00 (Migrated from github.com)
Copy Link

Migrated to Space

Migrated to Space

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
altavir
Labels
Clear labels   
bug

Something isn't working

  
dependencies

Pull requests that update a dependency file

  
discussion

   
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
feature

A new feature request

  
good first issue

The issue awaits its hero. Contributions are welcome

  
misc

Infrastructure tasks, cosmetic changes and miscellaneous tasks

  
performance

Performance optimization issue

  
question

Further information is requested

  
test

Add tests for some functionality

  
use case

A use case for library or feature

  
wontfix

This will not be worked on

No Label
bug
dependencies
discussion
documentation
duplicate
feature
good first issue
misc
performance
question
test
use case
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: kscience/kmath#96
No description provided.
Delete Branch "dev"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?