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

Reset BS/IAS15 in TRACE to reduce file size #795

Merged
merged 1 commit into from
Aug 27, 2024
Merged

Reset BS/IAS15 in TRACE to reduce file size #795

merged 1 commit into from
Aug 27, 2024

Conversation

hannorein
Copy link
Owner

It would be nice to reduce the file size in TRACE integration.

@tigerchenlu98, I naively thought these changes should not have any effects on the actual integration. But the units tests are failing. Do you understand why?

@hannorein
Copy link
Owner Author

Sorry, my bad: the unit tests pass! Do you see any downside coming from these changes? There should only be a negligible overhead in terms of runtime because the reset functions were already called at the beginning and calling them twice doesn't take long. But you have all these timing tests set up, so maybe you can verify that this is indeed the case?

@tigerchenlu98
Copy link
Contributor

Hey Hanno,

I see no reason the actual integration should change.

In terms of overhead, the only thing I'm concerned about is reallocing the nbody ode every time we call reset. I'm not sure if this would actually be an issue, but it's easily tested -- I'll run them and get back to you.

@tigerchenlu98
Copy link
Contributor

I ran the tests for all three pericenter modes -- integrations were all completely identical and runtimes were not meaningfully longer (less than 1 percent), so I think this is good to go!

@hannorein
Copy link
Owner Author

We already call reset at the beginning of the step, and then reallocating everything, so I don't think there's a significant overhead. And your tests seem to confirm that! Thanks! I'll merge it in...

@hannorein
Copy link
Owner Author

Fixes #794

@hannorein hannorein merged commit 5716b9a into main Aug 27, 2024
56 checks passed
@hannorein hannorein deleted the trace_reset branch August 29, 2024 13:13
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