-
Notifications
You must be signed in to change notification settings - Fork 29
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
#368 modify iterate method to allow timestep control #387
base: master
Are you sure you want to change the base?
Conversation
…or timestep control'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect! The only other thing I was going to ask is to use it in one of the examples or tests - just so that the create_iterator()
is explicitly called somewhere in CI
thetis/solver2d.py
Outdated
export_func=None): | ||
""" | ||
Creates a generator to iterate through the simulation and return access | ||
to time advancing function when time control is handled eternally. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo:
to time advancing function when time control is handled eternally. | |
to time advancing function when time control is handled externally. |
thetis/solver2d.py
Outdated
|
||
.. code-block:: python | ||
|
||
for t in solver_obj.generator(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for t in solver_obj.generator(): | |
for t in solver_obj.create_iterator(): |
thetis/solver2d.py
Outdated
|
||
.. code-block:: python | ||
|
||
thetis_timestepper = solver_obj.generator() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thetis_timestepper = solver_obj.generator() | |
thetis_timestepper = solver_obj.create_iterator() |
@@ -87,6 +89,7 @@ | |||
|
|||
# The velocity field is set up using a simple analytic expression. :: | |||
|
|||
x, y = SpatialCoordinate(mesh2d) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, that's a good demo to showcase the use of the iterator but it brings up another issue. Apparently the demo's aren't tested - demonstrated by the fact that you had to add this line - which is bad! See #389. The reason I asked for the create_iterator()
method to be used in a example or test, is precisely because we intend it to be public API so we want to know when it breaks.
closes #368
Implements the 2nd option proposed by @stephankramer in #368 by splitting
iterate
into two parts:create_iterator
- which gives a facelift refactor to the existingiterate
functionality as a generatoriterate
- which wrapscreate_iterator
and preserves the existing accessibility/structure