Exact Finish Time

[hannorein]

Currently, the exact finish time variable is inconsistent between python and c. Once again. Certainly my fault.

The question is how to choose the best default behaviour. Right now c has it set to 0 and python to 1.

Idea?

[dtamayo]

I have mixed feelings. On the one hand you want to protect the user in cases like using WHFast where you don’t want exact times. On the other, it can take a long time and be quite frustrating to figure out that the program is not by default giving you the exact times you ask for. I think you convinced me of the latter point of view last time. Maybe one possibility would be to add exact_time = 0 behavior in whfast_safe_mode?

On Aug 19, 2015, at 2:06 PM, Hanno Rein notifications@github.com wrote:

Currently, the exact finish time variable is inconsistent between python and c. Once again. Certainly my fault.

The question is how to choose the best default behaviour. Right now c has it set to 0 and python to 1.

Idea?

— Reply to this email directly or view it on GitHub https://github.com/hannorein/rebound/issues/73.

[dtamayo]

The more I've tried different things, the more you've convinced me that the right behavior should be exact_finish_time = 1. If you ask to integrate to t=time, that's what you should get. I think it should just show up prominently in the WHFast documentation that you probably want to set exact_finish_time = 0. What do you think?

[hannorein]

I tend to agree. We'd need to go through all the tutorials and examples to make sure it's consistent.

[dtamayo]

OK. That would also give me a reason to go through the ipython examples and change the calculate_orbits to use the new particles[i].element format. Unless you have an objection to that, do you want to split it up—I’ll do the ipython examples and you can do the C ones?

Also, the reason we had calculate_orbits in simulation.py was because you couldn’t do jacobi coordinates from within particle.calculate_orbit, since it needed to know about the other particles. Now we don’t have that limitation, and I think it makes sense to compartmentalize that functionality within the particle class. Then if you had a thousand planetesimals and a planet, you could calculate the elements of the planet all at once, but without doing the entire simulation. Would you agree with that? If so, I can remove sim.calculate_orbits and update all the ipython examples at once.

On Oct 23, 2015, at 11:29 AM, Hanno Rein notifications@github.com wrote:

I tend to agree. We'd need to go through all the tutorials and examples to make sure it's consistent.

— Reply to this email directly or view it on GitHub https://github.com/hannorein/rebound/issues/73#issuecomment-150608133.

[hannorein]

Hm. I need to think about the second part.

[dtamayo]

OK, I could be wrong about that. It just seems to me like if the state of the particle is in the Particle class, the functionality to convert that state to orbital elements should also be in there. Right now we have a function that the user isn’t supposed to call (particle.calculate_orbit) and one they should (sim.calculate_orbits()). This seems like it would be more flexible by letting the user use it like calculate orbits:

orbits = [sim.particles[i].calculate_orbit() for i in range(1,sim.N)]

or just get an individual orbit = sim.particles[3].calculate_orbit().

I’m happy to leave it as is though—just thought I’d bring it up while we were thinking about this part of the code.

On Oct 23, 2015, at 11:53 AM, Hanno Rein notifications@github.com wrote:

Hm. I need to think about the second part.

— Reply to this email directly or view it on GitHub https://github.com/hannorein/rebound/issues/73#issuecomment-150614776.

[hannorein]

Maybe a compromise would be a solution that keeps the old API unchanged. We could transform simulation.calculate_orbits() into a convince routine that just calls particle.calculate_orbit() on all particles (what you have written above).

[hannorein]

But note that this would be a bit slower because we need to recalculate the COM for every particle. Not sure this really matters though.

[dtamayo]

OK, I don’t think it’s a big deal. I’ll leave it as is. Now that I think about it more, it does occur to me that preserving old API is worthwhile for people who don’t follow all the changes like us, and find that their code doesn’t run all of a sudden!

On Oct 23, 2015, at 2:05 PM, Hanno Rein notifications@github.com wrote:

But note that this would be a bit slower because we need to recalculate the COM for every particle. Not sure this really matters though.

— Reply to this email directly or view it on GitHub https://github.com/hannorein/rebound/issues/73#issuecomment-150649827.

[hannorein]

So just to confirm, I'll be updating the C example to work with exact_finish_time=1 as a default.

[dtamayo]

Yes, and I’ll do the same for the ipython examples. During that process, would you like me to change the sim.calculate_orbits() sections to use the particles[i].elements syntax, or keep it as is? I’m happy with either.

On Oct 23, 2015, at 3:07 PM, Hanno Rein notifications@github.com wrote:

So just to confirm, I'll be updating the C example to work with exact_finish_time=1 as a default.

— Reply to this email directly or view it on GitHub https://github.com/hannorein/rebound/issues/73#issuecomment-150665552.

[hannorein]

Ok. I'm also happy with either. Whatever you prefer.

[hannorein]

So changing the c part was trivial. None of the examples really needed to be changed. I added a few extra comments here and there. All on branch exact_finish_time.