<br><div class="gmail_quote">On Thu, May 14, 2009 at 3:36 PM, Barry Smith <span dir="ltr"><<a href="mailto:bsmith@mcs.anl.gov">bsmith@mcs.anl.gov</a>></span> wrote:<br><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
  There is no one who is coordinating the TS interface at the moment. So you are welcome to propose changes/fixes/improvements and are in as good as a position as any of us.<br>
<br>
   I see there are four methods: prestep, poststep, update, postupdate; as you noticed the naming and usage of them is a bit inconsistent. Even the TSStep() function has a confusing name since it actually takes (potentially) many steps.  I suggest starting the following possible changes.<br>

<br>
   change TSStep to <span class="__mozilla-findbar-search" style="padding: 0pt; background-color: yellow; color: black; display: inline; font-size: inherit;">TSSolve</span>(),</blockquote><div><br>Yup, that makes a lot of sense. Actually, there is a <span class="__mozilla-findbar-search" style="padding: 0pt; background-color: yellow; color: black; display: inline; font-size: inherit;">TSSolve</span>() already, which however seems rarely used (all of the examples use TSStep()) <br>
<br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
   move the prestep() and poststep() methods to be the ones used AT the beginning and end of each timestep (not called in <span class="__mozilla-findbar-search" style="padding: 0pt; background-color: yellow; color: black; display: inline; font-size: inherit;">TSSolve</span>()) </blockquote>
<div><br>Agreed. I don't really like changing the behavior of functions while keeping the name the same, but I'll just assume there are few users of the current PreStep / PostStep, since they don't really make much sense. <br>
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br>
   eliminate update() and postupdate() confusing names anyways<br>
   determine if there is a need for a pre and post called in <span class="__mozilla-findbar-search" style="padding: 0pt; background-color: yellow; color: black; display: inline; font-size: inherit;">TSSolve</span>() could call them presolve and postsolve if you put them in.</blockquote>
<div><br>I don't think there's a need for a user-settable presolve/postsolve, because if they want to call functions before / after calling <span class="__mozilla-findbar-search" style="padding: 0pt; background-color: yellow; color: black; display: inline; font-size: inherit;">TSSolve</span>(), they can just call them directly. There may well be good reasons to introduce methods like this into TS that can be set by a specific implementation (like euler/beuler/...) to simplify the code there.<br>
 </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">do we really want prestep and poststep deciding to kick out of the iteration or do we want yet another method (corresponding to the converged() method in KSP and SNES) that is used to exit early from the solver?<br>

   I think using a flag to exit early from the solver is better than setting dt to zero.</blockquote><div><br>A new method seems like a much cleaner way, and would avoid breaking the existing method by changing the call sequence. Now the next question is for a good name. In some sense, "SetConvergenceTest" is nice because it's the same as used elsewhere, but it really doesn't quite describe what it really does here. So how about "SetTerminationTest"?<br>
 </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br>
   having the steps and ptime output arguments from <span class="__mozilla-findbar-search" style="padding: 0pt; background-color: yellow; color: black; display: inline; font-size: inherit;">TSSolve</span>() I think is wrong. They should be kept in the TS object and then calls like TSGetSolveTime() and TSGetNumberSteps()<br>

   to access them.</blockquote><div><br>I'm not sure what you're referring to. For TSMonitor(), I don't think there's a good reason to change the calling sequence. For TSStep() -> <span class="__mozilla-findbar-search" style="padding: 0pt; background-color: yellow; color: black; display: inline; font-size: inherit;">TSSolve</span>(), I definitely agree, <span class="__mozilla-findbar-search" style="padding: 0pt; background-color: yellow; color: black; display: inline; font-size: inherit;">TSSolve</span>() shouldn't return those values. I think the <span class="__mozilla-findbar-search" style="padding: 0pt; background-color: yellow; color: black; display: inline; font-size: inherit;">TSSolve</span>(TS ts, Vec sol) interface as it already exists is just fine.<br>
<br>There's already TSGetTime() and TSGetTimeStepNumber(), which can be used to do the above. They don't have the most elegant names, but I think we both believe that one shouldn't have multiple names for the same functionality, and I personally don't like renaming functions just for aesthetics because it's a pain to change applications and destroy backwards compatibility. (but past experience makes me think that the petsc developers look at this differently... ;)<br>
<br>My transition plan looks like this:<br>- Integrate the current TSStep() into <span class="__mozilla-findbar-search" style="padding: 0pt; background-color: yellow; color: black; display: inline; font-size: inherit;">TSSolve</span>() and remove TSStep from the public interface (which requires<br>
changing current users of TSStep()<br>- Rename the TSStep_* methods -> <span class="__mozilla-findbar-search" style="padding: 0pt; background-color: yellow; color: black; display: inline; font-size: inherit;">TSSolve</span>_*<br>
- Add a generic <span class="__mozilla-findbar-search" style="padding: 0pt; background-color: yellow; color: black; display: inline; font-size: inherit;">TSSolve</span>_TSStep() function, that provides a default timeloop implementation so that simple timestepping methods only have to implement a new TSStep_* method which does one (complete) step, but the actual looping, bookkeeping etc is handled by generic code. This could end up being an interface much like what Lisandro has to python. On the other hand, more complicated time integrators could take full control of the entire integration. (If it turns out there are no users left for such an interface, there may be no point in keeping it)<br>
<br>So actually I came across one point I'm wondering:<br>All current time steppers seem to do<br><br> VecCopy(sol, update);<br>while (!done) {<br>  FindSolutionAtNewTime(update);<br>  VecCopy(update,sol);<br>}<br><br>
Obviously for methods where steps may fail and will be retried, it's necessary to preserve the old solution, however<br>many schemes just call KSPSolve(.., update) or SNESSolve(..., update). In a linear method, I suppose by definition the operator does not depend on the current "sol". In a nonlinear problem, it will, however I suppose that SNESSolve() wouldn't update the solution vector until the very end, or am I missing something?<br>
<br>Point being, all these VecCopy()'s aren't really necessary and actually end up making the code a bit more complicated than needs be, and obviously aren't helping performance. (For implicit methods, those copies will generally be negligible, but for the explicit ones (euler eg), that's not so clear...)<br>
<br>--Kai<br><br></div></div>-- <br>Kai Germaschewski<br>Assistant Professor, Dept of Physics / Space Science Center<br>University of New Hampshire, Durham, NH 03824<br>office: Morse Hall 245F<br>phone:  +1-603-862-2912<br>
fax: +1-603-862-2771<br><br>