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

Timestepping #510

Merged
merged 8 commits into from
Apr 17, 2015
Merged

Timestepping #510

merged 8 commits into from
Apr 17, 2015

Conversation

hadjimy
Copy link
Member

@hadjimy hadjimy commented Apr 13, 2015

This is a short PR that improves the time-stepping procedure in pyclaw.

It includes the following:

  1. Move the selection of step size into step() routine. Now a single function (get_dt_new) is used to update the step size for all sharpclaw solvers.
  2. Move before_step attribute to base solver.
  3. Remove redundant register from SSP104 method; only a single register is required.
  4. Add SSP22 method and use it as a starting procedure for SSPLMMs.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 27.55% when pulling 19b183d on hadjimy:timestepping into e34ec5f on clawpack:master.

@@ -163,7 +178,8 @@ def __init__(self,riemann_solver=None,claw_package=None):
self._is_set_up = False
self._use_old_bc_sig = False
self.accept_step = False
self.adjust_dt = False
self.get_dt = False
self.before_step = before_step
Copy link
Member

Choose a reason for hiding this comment

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

I believe this should be initialized to None to avoid wasting time on needless function calls.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is now initialized to None.

- Avoid dublicating code for selection of dt.
- SSP22 SSP104 methods and get_dt routine do not return any variables.
- Rename dq_dt to self.dq_dt and avoid computing function evaluation
  if previous step is rejected.
- Rename self.dq_dt to self.prev_dq_dt_values.
- self.before_step is initialized to None.
- Use self.dt_old to check if we are at the first step or not.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 27.44% when pulling f24260f on hadjimy:timestepping into e34ec5f on clawpack:master.

@ketch
Copy link
Member

ketch commented Apr 16, 2015

This looks good to me now. @mandli since this is an internal refactoring and doesn't change anything that the average user should touch, is it okay with you if we sneak it into 5.3.0? If you think it is better we can wait for 5.3.1.

@mandli
Copy link
Member

mandli commented Apr 16, 2015

I think that's fine with me. I would say merge this.

ketch added a commit that referenced this pull request Apr 17, 2015
@ketch ketch merged commit cfa679b into clawpack:master Apr 17, 2015
@ketch ketch removed the in progress label Apr 17, 2015
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.

None yet

4 participants