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

A few bugs & an extra feature #749

Closed

Conversation

tigerchenlu98
Copy link
Contributor

Hi @hannorein, don't merge this yet, just pointing out a few things I'm currently looking into:

  1. Re-added the ipython example HybridIntegrationsWithTRACE. I was using the same outer solar system x50 example that MERCURIUS used. I'd run this system before and it worked, but now we're doing a lot worse than MERCURIUS. All the other tests run fine, so not immediately clear what's going on.

  2. Speaking of unit tests, the Harmonic Oscillator one fails now too. The test was originally commented out (must have done that on accident), but I'd run it previously and it was fine. Maybe something to do with the new handling of the BS step?

  3. A new feature - David and I are realizing that collisions with the central object may not always be correctly handled for test particles, because they don't (and shouldn't) trigger the pericenter condition. My thinking is the best way to handle this is just to add a collision_search at the end of each timestep and modifying to work with mode=0, but let me know if you have thoughts on that.

@hannorein
Copy link
Owner

The version on the newTRACE4 branch is nowhere near ready to be used. Neither additional ODEs nor pericenter approach work.

I was hoping to not add the example to git just yet, especially the ipynb ones because it makes diff-ing so much harder.

Are you sure you don't want to trigger the pericenter condition for test particles? I think the argument is similar to the one about KL cycles. You're not resolving the shortest timescales in the problem if you don't trigger the condition. That might be fine in many cases. Maybe a short meeting could resolve any disagreement/confusion. FWIW, I think we should either remove the KL example from the paper, or use it as a counter example as where to not use TRACE.

@tigerchenlu98
Copy link
Contributor Author

Understandable, I'll get rid of & hold off on examples for now.

I was going to wait and see if IAS15 improves the Kozai example at all. I don't think it will because it seems to be more of a failing of DHC, but there's no denying that BS as a whole is performing poorly for this system so maybe... if there is no improvement then I'll certainly rework it into a fail case for TRACE.

Happy to discuss the pericenter condition issue - I did some more testing with the accretion example which makes me lean toward no pericenter to TPs, since it's a significant speedup.

@hannorein
Copy link
Owner

Closing this one in favour of #771.

@hannorein hannorein closed this May 5, 2024
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