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

Typo in documentation of opty #297

Open
Peter230655 opened this issue Jan 10, 2025 · 7 comments
Open

Typo in documentation of opty #297

Peter230655 opened this issue Jan 10, 2025 · 7 comments

Comments

@Peter230655
Copy link
Contributor

In the opty documentation release 1.4.0.dev0 on page 129 it says:
jacobian(free)
Returns.....
Return type
ndarray.shape((2n + q + r + s) * (n(N-1) + o ), )
but in my test it returns
ndarray.shape((2n + q + r + s) * (n(N-1)) + o, ) The bracket to the right of o is moved to the left.
@moorepants confirmed this is a typo.

@moorepants
Copy link
Member

The docstring for functions are in the source file right under the associated function signature. The line you mention here in Problem.jacobian() is here: https://github.com/csu-hmc/opty/blob/master/opty/direct_collocation.py#L343

@Peter230655
Copy link
Contributor Author

I start to see it. (I thought naively that the documentation was some separate document, maybe written in over-leaf or so.)
So, if I want to change this ")" I change it and make a regular PR, right?

@Peter230655
Copy link
Contributor Author

Peter230655 commented Jan 18, 2025

Question, just to verify, I start to understand how the documentation is generated:

  • opty's class Problem inherits from cyipopt.Problem, amongst others the method solve(...). So the description of solve in opty's documentation is taken from the docstrings of that solve in cyipopt.Problem.
  • So, in order to change the description of solve on page 132 of the opty documentation (which is not totally consistent with the rest in the documentation, I feel), one would have to somehow change the description in the docstring of cyipopt.Problem.solve(...)

Am I basically on the right track?

@moorepants
Copy link
Member

It depends. cyipopt.Problem documentation should stand indedpendent of opty, but if there is an error in cyipopt or you want to expand the general explanations then you edit cyipopt. If it is something opty specific and you want a different docstring in opy for .solve() than the cyipopt.Problem.solve() we can overwrite the cyipopt docstring inside of opty (only).

To overwrite it you do something like:

class Problem(cyipopt.Problem):
    def solve(self, ...):
        """Override docstring""""
        return super().solve(...)

@Peter230655
Copy link
Contributor Author

Peter230655 commented Jan 19, 2025

In the opty documentation there is this explanation, actually at a few places:

Image

Then, a bit below, solve is explained:

Image

The shape of x given as (n, ) I think is not correct as per the definition of n above.
I am likely not the smartest user of opty, so I cannot really judge whether this should be changed,
@moorepants if you think, it should be changed, I will try as per your suggestion.

@moorepants
Copy link
Member

That is a good reason to overwrite the .solve() docstring inside opty (only), as it is correct for cyipopt.

@Peter230655
Copy link
Contributor Author

I will try it.

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

No branches or pull requests

2 participants