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

DOC: fix doxstring for compte_sequence in lqcontrol #157

Closed
wants to merge 6 commits into from

Conversation

sglyon
Copy link
Member

@sglyon sglyon commented Jun 22, 2015

Wasn't true!

@mmcky
Copy link
Contributor

mmcky commented Aug 25, 2015

@spencerlyon2 Just processing all open PRs. Is this still an issue?

MarkovChain.simulate API change; random_state option added
@sglyon
Copy link
Member Author

sglyon commented Aug 26, 2015

Not sure. I'd have to look again. Want me to check it out?

@mmcky
Copy link
Contributor

mmcky commented Aug 27, 2015

@spencerlyon2 there aren't many changes. It looks like you have converted to using np.random.randn rather than some dot function in addition to updating the docstring.

@sglyon
Copy link
Member Author

sglyon commented Aug 27, 2015

I think it is still meaningful. In the docstring we claimed to be returning the w_t, but we were actually returning C * w_t

Also the number of elements in the return value was incorrectly documented.

@mmcky
Copy link
Contributor

mmcky commented Aug 27, 2015

Cool - I will replicate the changes and push them.

On Thu, Aug 27, 2015 at 1:39 PM, Spencer Lyon notifications@github.com
wrote:

I think it is still meaningful. In the docstring we claimed to be
returning the w_t, but we were actually returning C * w_t

Also the number of elements in the return value was incorrectly
documented.


Reply to this email directly or view it on GitHub
#157 (comment)
.

@mmcky
Copy link
Contributor

mmcky commented Aug 31, 2015

@spencerlyon2 I rebased this change but it is failing the test.

 ======================================================================
FAIL: test_scalar_sequences (quantecon.tests.test_lqcontrol.TestLQControl)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/QuantEcon/QuantEcon.py/quantecon/tests/test_lqcontrol.py", line 61, in test_scalar_sequences
    assert_allclose(x_1, x_seq[0, -1], rtol=1e-4)
  File "/home/travis/miniconda/lib/python2.7/site-packages/numpy/testing/utils.py", line 1297, in assert_allclose
    verbose=verbose, header=header)
  File "/home/travis/miniconda/lib/python2.7/site-packages/numpy/testing/utils.py", line 665, in assert_array_compare
    raise AssertionError(msg)
AssertionError: 
Not equal to tolerance rtol=0.0001, atol=0
(mismatch 100.0%)
 x: array([[ 0.146818]], dtype=float32)
 y: array(0.932981936024652)

@sglyon
Copy link
Member Author

sglyon commented Sep 1, 2015

@mmcky sorry, I don't really have time to track down the error.

@mmcky mmcky closed this Apr 19, 2017
sglyon added a commit that referenced this pull request Apr 20, 2017
implement changes from PR #157 to fix lqcontrol docstring
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