Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor RandomVariable #164

Conversation

marvinpfoertner
Copy link
Collaborator

@marvinpfoertner marvinpfoertner commented Aug 20, 2020

In essence, this PR is a substantial refactor of the RandomVariable and Distribution classes that:

  • gets rid of Distributions and integrates their functionality into the RandomVariable class (closes Remove Distribution class and only use RandomVariable with subclasses #131)
  • creates a corresponding RandomVariable subclass for every Distribution subclass (closes Remove Distribution class and only use RandomVariable with subclasses #131)
  • gets rid of the probnum.prob module and introduces the new probnum.random_variables module
  • introduces a functional/compositional pattern in RandomVariable and Normal to allow for flexible and efficient recombination of sampling, mean/cov/var/entropy computation, and (log){cd,pf,pm}f evaluation (closes Refactor Normal class to compositional pattern #145)
  • introduces a flexible way of implementing binary arithmetic between random variables and provides generic fallback operations for unary and binary arithmetic and for reshaping, transposition, (advanced) indexing, slicing and masking of random variables
  • introduces cached_property decorators for all random variable properties in order to save computation time in case the distribution of the random variable is implicity given, e.g. via an MCMC sampler
  • introduces type hints for the new RandomVariable class, its subclasses and all related parts of the API. It also introduces typing infrastructure in the probnum.typing and probnum.utils.typeutils modules (addresses Type Hints #61)
  • fixes almost all pylint messages that had to be disabled in Refactor probnum.randvars to resolve the pylint messages #159. The remaining disabled message will be fixed in a follow-up documentation PR

This should also fix the bug about the sampling shapes, i.e. #57, but this is not tested yet.

Many of the changes are not yet documented properly. This will be done in a follow-up PR by @JonathanWenger with my support.

benchmarks/distribution.py Outdated Show resolved Hide resolved
src/probnum/diffeq/ode/ivp.py Outdated Show resolved Hide resolved
src/probnum/diffeq/odefiltsmooth/ivp2filter.py Outdated Show resolved Hide resolved
src/probnum/prob/_randomvariablelist.py Outdated Show resolved Hide resolved
tests/test_diffeq/test_odefiltsmooth/test_prior.py Outdated Show resolved Hide resolved
src/probnum/prob/random_variable/__init__.py Outdated Show resolved Hide resolved
src/probnum/utils/randomutils.py Show resolved Hide resolved
src/probnum/prob/random_variable/_dirac.py Outdated Show resolved Hide resolved
src/probnum/prob/_random_variable.py Outdated Show resolved Hide resolved
src/probnum/prob/_random_variable.py Outdated Show resolved Hide resolved
@JonathanWenger
Copy link
Contributor

JonathanWenger commented Aug 20, 2020

In order to merge this PR the following TODOs need to be resolved:

  • Update benchmarks (in particular the folder named distribution) to a more appropriate naming scheme
  • Ensure all notebooks work, have been re-run and use the way we intend users to import Dirac, Normal, etc.
  • Update the quickstart guide and notebooks explaining the use of random variables
  • Update the documentation thoroughly in all changed files

@codecov
Copy link

codecov bot commented Aug 21, 2020

Codecov Report

Merging #164 into master will increase coverage by 0.61%.
The diff coverage is 67.32%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #164      +/-   ##
==========================================
+ Coverage   73.49%   74.10%   +0.61%     
==========================================
  Files          58       61       +3     
  Lines        3060     3167     +107     
  Branches      393      409      +16     
==========================================
+ Hits         2249     2347      +98     
+ Misses        660      659       -1     
- Partials      151      161      +10     
Impacted Files Coverage Δ
src/probnum/filtsmooth/bayesfiltsmooth.py 81.48% <0.00%> (ø)
src/probnum/filtsmooth/statespace/statespace.py 100.00% <ø> (ø)
src/probnum/utils/arrayutils.py 42.55% <25.00%> (ø)
src/probnum/linalg/linearsolvers/matrixbased.py 57.14% <30.00%> (+0.10%) ⬆️
src/probnum/linalg/linearsolvers/linearsolvers.py 46.60% <36.36%> (ø)
src/probnum/_randomvariablelist.py 78.94% <50.00%> (ø)
src/probnum/diffeq/odefiltsmooth/ivp2filter.py 80.32% <50.00%> (-0.16%) ⬇️
src/probnum/random_variables/_random_variable.py 55.78% <55.78%> (ø)
src/probnum/utils/argutils.py 60.00% <60.00%> (ø)
src/probnum/utils/randomutils.py 69.23% <69.23%> (ø)
... and 32 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a232a03...aac484b. Read the comment docs.

@pnkraemer pnkraemer mentioned this pull request Aug 26, 2020
@marvinpfoertner marvinpfoertner force-pushed the refactor/random-variable branch 3 times, most recently from a765fe8 to 1438bc9 Compare August 28, 2020 07:04
@marvinpfoertner marvinpfoertner changed the title [WIP] Refactor RandomVariable Refactor RandomVariable Aug 28, 2020
@marvinpfoertner marvinpfoertner marked this pull request as ready for review August 28, 2020 10:34
@marvinpfoertner marvinpfoertner force-pushed the refactor/random-variable branch 2 times, most recently from 9414231 to 263a534 Compare August 28, 2020 10:54
tests/test_random_variables/test_normal.py Show resolved Hide resolved
tests/test_random_variables/test_normal.py Outdated Show resolved Hide resolved
tests/test_random_variables/test_normal.py Outdated Show resolved Hide resolved
src/probnum/random_variables/_dirac.py Outdated Show resolved Hide resolved
src/probnum/random_variables/_scipy_stats.py Show resolved Hide resolved
src/probnum/random_variables/_utils.py Show resolved Hide resolved
src/probnum/random_variables/_normal.py Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
randvars Issues related to random variables refactoring Refactoring of existing functionality
Projects
None yet
3 participants