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

adjusted docstrings as per #348, reduced execution time #377

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

Peter230655
Copy link
Contributor

While the program was designerd to run both versions, fixed time interval and variable interval, only the animation for variable interval was shown. I commented out the fixed time interval portions. Reduced execution time on my PC from around 53 sec to around 23 sec.


# %%
prob_list[selection].plot_objective_value()
#_ = prob_list[selection].plot_objective_value()
Copy link
Member

Choose a reason for hiding this comment

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

Don't comment out lines in examples, best to remove them vs. commenting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't comment out lines in examples, best to remove them vs. commenting.

I thought like this a potential user could easily return to running both. I put in the Notes why I did it.
Still better to delete

Copy link
Member

@moorepants moorepants Feb 22, 2025

Choose a reason for hiding this comment

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

I think it is a bit bad form. If you want to tell them some kind of option that will not actually be run in our example do this in a comment block as rst:

# %%
# If you want to check ... then use code like::
# 
#    prob_list[selection].plot_objective_value()

This will show up as a monospaced text they can easily copy and paste into their script but will not be run in the example.

@Peter230655
Copy link
Contributor Author

does this look better?
(Actually now I see, why it is better than commented out)

@Peter230655
Copy link
Contributor Author

This simulation seems to be free of the f string error

#
# fig, ax = plt.subplots(figsize=(8, 8))
#
# anim, _ = drucken(selection, fig, ax)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this will render as you expect. To do code blocks in RST format it looks like:

Type this code::

   a, b = 1, 2
   print(a + b)

Copy link
Contributor Author

@Peter230655 Peter230655 Feb 23, 2025

Choose a reason for hiding this comment

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

I played around with it for a long time. Then, finally, on the make html rendering it looked o.k.
I ran out of ideas.
I start like this:
# %%
and the added the text.

I don't think this will render as you expect. To do code blocks in RST format it looks like:

Type this code::

   a, b = 1, 2
   print(a + b)

Does this mean I have to typle like
three back slashes
text
three backslashes

Copy link
Member

Choose a reason for hiding this comment

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

All the code lines should be indented relative to the text line above it that ends in a ::.

https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#literal-blocks

@@ -105,7 +124,13 @@ def strasse(x, a, b):

# %%
# Set up the optimization problems and solve them.
for selection in (0, 1):
# if you want to run both optimizations, replace the two lines below with this
# line:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# line:
# line::

# if you want to run both optimizations, replace the two lines below with this
# line:
#
# for selection in [0, 1]:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# for selection in [0, 1]:
# for selection in [0, 1]:

Copy link
Member

Choose a reason for hiding this comment

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

Those are the two fixes for this to render correct (I think).

fig, ax = plt.subplots(figsize=(8, 8))
anim, _ = drucken(selection, fig, ax)
# If you want to run the solution with a fixed time interval, you should add the
# following code to the code here:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# following code to the code here:
# following code to the code here::

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!!
I will be out of station for two days, will try on Tuesday.

Copy link
Member

Choose a reason for hiding this comment

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

There is no rush on anything.

Objective
---------

- a simplex example to show how to use ``opty's`` capability of variabel node
Copy link
Member

@moorepants moorepants Feb 24, 2025

Choose a reason for hiding this comment

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

Suggested change
- a simplex example to show how to use ``opty's`` capability of variabel node
- a simple example to show how to use opty's capability of variable node

@Peter230655
Copy link
Contributor Author

using backend='numpy' reduced running time again from about 27 sec to 16 sec on my PC.

@Peter230655
Copy link
Contributor Author

Error said unexpected indentation in line 309, but there was on indentation that I could see.

@Peter230655
Copy link
Contributor Author

Peter230655 commented Feb 25, 2025

@moorepants I am at a loss with the error message:
image

my code looks like this:
image

I see no indentation.
Thanks for a hint!

@Peter230655
Copy link
Contributor Author

There is an error unexpected indentation which I do not understand. So all I can do is try different things and keep on pushing.

@Peter230655
Copy link
Contributor Author

Peter230655 commented Feb 27, 2025

I am at a loss with this error of unexpected indentation
When I look at docs/readthedocs.org:opty it seems to build fine...

@moorepants
Copy link
Member

It is a RestructuredText formatting error.

@Peter230655
Copy link
Contributor Author

It is a RestructuredText formatting error.

So, not my error?

@moorepants
Copy link
Member

Yes, it is your formatting error. Anything in a # %% "cell" is processed by RestructureText.

@Peter230655
Copy link
Contributor Author

Peter230655 commented Feb 27, 2025

Yes, it is your formatting error. Anything in a # %% "cell" is processed by RestructureText.

it objects to line 309:
image

I cannot see, where 309 is different from 301 or 305.
Does it want a # %% above line 314?

@moorepants
Copy link
Member

It isn't line 309 in the py file it is line 309 in the generated rst file: plot_sliding_block.rst

@Peter230655
Copy link
Contributor Author

It isn't line 309 in the py file it is line 309 in the generated rst file: plot_sliding_block.rst

Thanks! At least now I got in the area of the problem.

@Peter230655
Copy link
Contributor Author

Peter230655 commented Feb 28, 2025

Reduced running time from 17 sec to 8 sec, by reducing frames in the animation. It deteriorates the animation a bit.

@Peter230655
Copy link
Contributor Author

Peter230655 commented Feb 28, 2025

It objects to worng indentation in line 309 of the rst file.
I found the .rst file in opty/docs/example/beginner on my PC
It looks like this:
image

line 309 is empty, the other indentations look correct to me.
I do not know what else to do.
Thanks for any help!

@moorepants
Copy link
Member

The formatting rules for RestructuredText literal blocks is here: https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#literal-blocks

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