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

BUG: Correct derivative for exponential transform. #1540

Merged
merged 1 commit into from Apr 2, 2014

Conversation

Projects
None yet
2 participants
@jseabold
Copy link
Member

commented Apr 2, 2014

See if this fixes #1539.

@jseabold

This comment has been minimized.

Copy link
Member Author

commented Apr 2, 2014

It looks like it fixes the issue of score always matching numerical score, but this model is still bizarre. Might try comparing output to stata.

@jseabold

This comment has been minimized.

Copy link
Member Author

commented Apr 2, 2014

See #1539. It appears to be sensitive to starting values. You need to make sure alpha >= ~1 or lnalpah >= ~0.

@jseabold

This comment has been minimized.

Copy link
Member Author

commented Apr 2, 2014

Merge? Marking for 0.5.1.

@jseabold jseabold added this to the 0.5.1 milestone Apr 2, 2014

@josef-pkt

This comment has been minimized.

Copy link
Member

commented Apr 2, 2014

wait for TravisCI to report back, then merge

BTW don't forget to add labels, it looks like we can add them directly now also to PRs

@jseabold

This comment has been minimized.

Copy link
Member Author

commented Apr 2, 2014

Sure. Tests passed locally, which means this was untested. I think we only tested the default solves for which this wasn't such an issue.

@josef-pkt

This comment has been minimized.

Copy link
Member

commented Apr 2, 2014

I think this is only "half" a bug, and I thought it was tested.

At the optimum, the score is approximately zero. multiplying it by a factor doesn't make any difference, (except for convergence tolerance in score which, I think, we don't check.)
postestimation results where based on untransformed parameters and were also correct.

The only problem was in the optimization itself, especially further away from the optimum or with a messy case like this.

@jseabold

This comment has been minimized.

Copy link
Member Author

commented Apr 2, 2014

Makes sense. Still conceptually incorrect. Merging. Also marked to backport to 0.5.1. Need to go back and check those with milestone tags and no backport labels.

jseabold added a commit that referenced this pull request Apr 2, 2014

Merge pull request #1540 from jseabold/fix-1539
BUG: Correct derivative for exponential transform.

@jseabold jseabold merged commit 0fd2925 into statsmodels:master Apr 2, 2014

1 check passed

default The Travis CI build passed
Details

@jseabold jseabold deleted the jseabold:fix-1539 branch Apr 2, 2014

@josef-pkt josef-pkt added the PR label Apr 14, 2014

PierreBdR pushed a commit to PierreBdR/statsmodels that referenced this pull request Sep 2, 2014

Merge pull request statsmodels#1540 from jseabold/fix-1539
BUG: Correct derivative for exponential transform.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.