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

Fix DiscreteDstn #646

Merged
merged 6 commits into from
May 1, 2020
Merged

Fix DiscreteDstn #646

merged 6 commits into from
May 1, 2020

Conversation

mnwhite
Copy link
Contributor

@mnwhite mnwhite commented Apr 23, 2020

DiscreteDstn wasn't working properly with multivariate distributions. This PR makes some small fixes to it.

The new version of drawDiscrete did not work properly when the distribution was multivariate.  That's now fixed, returning a JxN array of draws (where J is the dimension of the distribution).

It also seemed to have a typo/error, referring to Xdim, which does not exist.

This commit does not make drawDiscrete work when the input X is an integer, but it didn't work before either.
Fixed makeAggShockDstn() to work properly with new format.

if exact_match:
# Set up the RNG
RNG = np.random.RandomState(seed)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused-- why did you move the RNG setting into exact_match?

@sbenthall
Copy link
Contributor

Ideally, this PR would come with an automated test catching the case that was originally broken.

That would be very helpful because I expect that the drawDiscrete API will change in the future as more work is done on the distribution handling.

@sbenthall sbenthall merged commit e532b78 into master May 1, 2020
@mnwhite
Copy link
Contributor Author

mnwhite commented May 1, 2020 via email

@sbenthall
Copy link
Contributor

I saw that. I figured it was better to have the fix in than not.
But a test case is always welcome--you can add it to the same branch and file it as a PR whenever you get to it.

@mnwhite mnwhite deleted the FixDiscreteDstn branch June 5, 2020 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants