`scipy.stats` distributions #1197

alanlujan91 · 2023-01-01T23:12:29Z

Please ensure your pull request adheres to the following guidelines:

Tests for new functionality/models or Tests to reproduce the bug-fix in code.
Updated documentation of features that add new functionality.
Update CHANGELOG.md with major/minor changes.

reset and generate new random seed; implement generic draw method; rng is now private attribute

create parametrized (frozen) distributions from scipy with seed management

create discrete scipy distributions with seed manager

codecov-commenter · 2023-01-01T23:27:54Z

Codecov Report

Base: 73.38% // Head: 73.28% // Decreases project coverage by -0.10% ⚠️

Coverage data is based on head (dd10e24) compared to base (881aa06).
Patch coverage: 91.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1197      +/-   ##
==========================================
- Coverage   73.38%   73.28%   -0.11%     
==========================================
  Files          74       74              
  Lines       12150    12149       -1     
==========================================
- Hits         8916     8903      -13     
- Misses       3234     3246      +12

Impacted Files	Coverage Δ
HARK/ConsumptionSaving/ConsMedModel.py	`73.99% <ø> (ø)`
HARK/ConsumptionSaving/ConsPortfolioFrameModel.py	`100.00% <ø> (ø)`
HARK/ConsumptionSaving/ConsPrefShockModel.py	`79.68% <ø> (ø)`
HARK/ConsumptionSaving/ConsIndShockModel.py	`86.40% <82.92%> (ø)`
HARK/distribution.py	`80.70% <90.38%> (-2.40%)`	⬇️
HARK/ConsumptionSaving/ConsAggShockModel.py	`89.64% <100.00%> (ø)`
HARK/ConsumptionSaving/ConsRiskyAssetModel.py	`41.03% <100.00%> (ø)`
...RK/ConsumptionSaving/tests/test_ConsMarkovModel.py	`86.56% <100.00%> (ø)`
...K/ConsumptionSaving/tests/test_modelcomparisons.py	`98.46% <100.00%> (ø)`
HARK/core.py	`91.48% <100.00%> (ø)`
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

set probability in from_dataset method

DiscreteDistribution returns self

llorracc · 2023-01-07T17:18:22Z

Only comment is that I'd like it if the invocations of .discretize (which replace the .approx) were to explicitly specify the method ("equiprobable").

alanlujan91 · 2023-01-07T19:40:21Z

Only comment is that I'd like it if the invocations of .discretize (which replace the .approx) were to explicitly specify the method ("equiprobable").

Ok, right now the default for every distribution is equiprobable except for Normal which has a default of hermite.

llorracc · 2023-01-09T03:07:13Z

OK, I'm happy with this now. (Actually, delighted).

Only two things I can think of:

Update the changelog
Figure out how to test it against the DemARKs BEFORE we merge into master
- If all the DemARKs break, we will either need to fix them or to change the code so it works with the old code (maybe issuing a deprecation warning)
- If only a few DemARKs break, they can be fixed

alanlujan91 · 2023-01-10T00:17:00Z

@llorracc the only way I can think of checking this branch against DemARK's is locally. I will try that and see what the results are

alanlujan91 · 2023-01-10T00:37:35Z

@llorracc seems like DemARK's already broke with previous merge so I'll have to go and fix them anyway

HARK/ConsumptionSaving/ConsIndShockModel.py

sbenthall · 2023-01-10T19:29:49Z

HARK/distribution.py

        """
        Construct a discrete approximation to a lognormal distribution with underlying
        normal distribution N(mu,sigma).  Makes an equiprobable distribution by
        default, but user can optionally request augmented tails with exponentially
        sized point masses.  This can improve solution accuracy in some models.

+        TODO: add endpoints option


Is there an issue for this in the issue tracker?

Are you going to do it in this PR, or leave it to a later release?

I was thinking of leaving it for a different PR since I still need to discuss how to do this with the group.

I will make an issue for this.

sbenthall · 2023-01-10T19:33:01Z

HARK/distribution.py

+            "tail_N": tail_N,
+            "tail_bound": tail_bound,
+            "tail_order": tail_order,
+        }


This may be implemented a little more generically using **kwargs to capture the arguments given the discretize method.

https://www.digitalocean.com/community/tutorials/how-to-use-args-and-kwargs-in-python-3

That's a good point. Will think about this and leave it for a different PR.

HARK/distribution.py

sbenthall · 2023-01-10T19:38:25Z

HARK/distribution.py

+        `DiscreteDistribution` is already an approximation, so this method
+        returns a copy of the distribution.
+
+        TODO: print warning message?


Is there an issue for this?

Will write an issue for this

sbenthall · 2023-01-10T19:40:44Z

Looking good! See review comment inline. Please update the CHANGELOG in this PR.

alanlujan91 · 2023-01-11T02:07:51Z

Changes in this PR:

Reorganize Distribution class as seed manager with abstract draw and discretize methods. Random number generator _rng is now a private attribute.
New discretize method replaces approx, with parameters N, method, endpoints, and additional keywords. The recommended use is to be explicit about the discretization method being used by setting dist.discretize(N, method = "EXPLICIT"). The endpoints parameter has only been implemented for Uniform distribution so far but it should allow user to add points with infinitesimal probability in the Discrete Distribution.
Create new ContinuousFrozenDistribution which allows users to use any scipy.stats continuous distribution on HARK even if they have not been implemented in HARK.
All HARK distributions now inherit methods from scipy.stats such as rvs, pdf, cdf, sf, ppf, isf, stats, moment.
New Lognormal.discretize(N, method = "hermite") which uses Gauss Hermite method to discretize Lognormal and MeanOneLogNormal.
Create new DiscreteFrozenDistribution which allows users to use any scipy.stats discrete distribution on HARK even if they have not been implemented in HARK.
DiscreteDistribution and DiscreteDistributionLabeled now take parameter limit to indicate the Continuous Distribution it comes from and the method used for discretization.
Implement the use of dist.discretize with explicit method throughout the codebase.

alanlujan91 added 20 commits December 16, 2022 18:42

Merge branch 'scipy_generator' into scipy_dist

b40ce36

Merge branch 'scipy_generator' into scipy_dist

f06b338

Merge branch 'scipy_generator' into scipy_dist

2b99a4f

Merge remote-tracking branch 'upstream/master' into scipy_dist

a8d996d

Merge remote-tracking branch 'upstream/master' into scipy_dist

f42a288

Merge remote-tracking branch 'upstream/master' into scipy_dist

1d2148e

reorganize distributions

e50ee7e

update Distribution as seed manager

85c2d12

reset and generate new random seed; implement generic draw method; rng is now private attribute

implement ContinuousFrozenDistribution

b279ec0

create parametrized (frozen) distributions from scipy with seed management

update Normal

76a5762

update Lognormal

fcc1116

update Uniform

1f50afd

update Weibull

0789215

update MVNormal

c21ed13

implement DiscreteFrozenDistribution

40ed24c

create discrete scipy distributions with seed manager

update Bernoulli

ae92315

update DiscreteDistribution

4cc6f72

update DiscreteDistributionLabeled

abc64e9

update IndexDistribution

dc5f46f

update MarkovProcess

7888fb8

alanlujan91 added 2 commits January 1, 2023 20:44

fix notebook bug

24cd88c

set probability in from_dataset method

abstract approx method

f44bfa0

DiscreteDistribution returns self

sbenthall added this to the 0.13.0 milestone Jan 4, 2023

sbenthall self-requested a review January 4, 2023 17:45

sbenthall assigned alanlujan91 and sbenthall Jan 4, 2023

alanlujan91 added 3 commits January 5, 2023 10:41

Merge remote-tracking branch 'upstream/master' into scipy_dist

9a65ffd

fix bugs to pass tests

dc6d7a4

update notebooks

43ed27b

alanlujan91 removed their assignment Jan 7, 2023

use 'expand_dims`

539722f

alanlujan91 added 4 commits January 7, 2023 18:21

add documentation

69e932e

set method parameter on each discretize call

1149b52

fix error

41d2bae

implement hermite approx to lognormal

7b22040

alanlujan91 added Ready-To-Merge Has been reviewed and when branch is updated and checks pass it should be merged Status: Review Needed and removed Ready-To-Merge Has been reviewed and when branch is updated and checks pass it should be merged labels Jan 9, 2023

alanlujan91 added 2 commits January 9, 2023 15:50

Merge remote-tracking branch 'upstream/master' into scipy_dist

54870bf

update failing notebook

dd10e24

sbenthall reviewed Jan 10, 2023

View reviewed changes

HARK/ConsumptionSaving/ConsIndShockModel.py Outdated Show resolved Hide resolved

sbenthall reviewed Jan 10, 2023

View reviewed changes

HARK/distribution.py Outdated Show resolved Hide resolved

sbenthall reviewed Jan 10, 2023

View reviewed changes

HARK/distribution.py Outdated Show resolved Hide resolved

sbenthall reviewed Jan 10, 2023

View reviewed changes

address Seb's comments

e2d62a1

Update CHANGELOG.md

05edaf7

alanlujan91 changed the title ~~[WIP] scipy.stats distributions~~ scipy.stats distributions Jan 11, 2023

alanlujan91 added the Ready-To-Merge Has been reviewed and when branch is updated and checks pass it should be merged label Jan 11, 2023

sbenthall merged commit 079839a into econ-ark:master Jan 11, 2023

sbenthall mentioned this pull request Jan 12, 2023

[WIP] Updates to code econ-ark/DistributionOfWealthMPC#4

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`scipy.stats` distributions #1197

`scipy.stats` distributions #1197

alanlujan91 commented Jan 1, 2023 •

edited

Loading

codecov-commenter commented Jan 1, 2023 •

edited

Loading

llorracc commented Jan 7, 2023

alanlujan91 commented Jan 7, 2023

llorracc commented Jan 9, 2023

alanlujan91 commented Jan 10, 2023

alanlujan91 commented Jan 10, 2023

sbenthall Jan 10, 2023

alanlujan91 Jan 11, 2023

alanlujan91 Jan 11, 2023

sbenthall Jan 10, 2023

alanlujan91 Jan 11, 2023

sbenthall Jan 10, 2023

alanlujan91 Jan 11, 2023

alanlujan91 Jan 11, 2023

sbenthall commented Jan 10, 2023

alanlujan91 commented Jan 11, 2023

scipy.stats distributions #1197

scipy.stats distributions #1197

Conversation

alanlujan91 commented Jan 1, 2023 • edited Loading

codecov-commenter commented Jan 1, 2023 • edited Loading

Codecov Report

llorracc commented Jan 7, 2023

alanlujan91 commented Jan 7, 2023

llorracc commented Jan 9, 2023

alanlujan91 commented Jan 10, 2023

alanlujan91 commented Jan 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbenthall commented Jan 10, 2023

alanlujan91 commented Jan 11, 2023

`scipy.stats` distributions #1197

`scipy.stats` distributions #1197

alanlujan91 commented Jan 1, 2023 •

edited

Loading

codecov-commenter commented Jan 1, 2023 •

edited

Loading