-
Notifications
You must be signed in to change notification settings - Fork 18
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
Add a configuration option to use integrate over integrate_or_interpolate #1034
base: main
Are you sure you want to change the base?
Add a configuration option to use integrate over integrate_or_interpolate #1034
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1034 +/- ##
==========================================
+ Coverage 80.90% 81.08% +0.17%
==========================================
Files 70 70
Lines 3148 3182 +34
==========================================
+ Hits 2547 2580 +33
- Misses 601 602 +1 ☔ View full report in Codecov by Sentry. |
This is great!!! One question before approving: which one is set by default? Integrate_or_interpolate or just integrate? |
Sorry, I haven't had time to look at this. I don't want to hold up the
works, but can you give me 2-3 days?
…On Mon, Oct 21, 2024 at 1:31 PM Pedro Bernardinelli < ***@***.***> wrote:
This is great!!! One question before approving: which one is set by
default? Integrate_or_interpolate or just integrate?
—
Reply to this email directly, view it on GitHub
<#1034 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA5CVWPJ5XDUYSX3MNRXPWLZ4U273AVCNFSM6AAAAABPXFITFGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMRXGMZDEMZTG4>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
--
Matthew J. Holman, PhD
Senior Astrophysicist
Center for Astrophysics | Harvard & Smithsonian
60 Garden Street, MS #51
Cambridge, MA 02138
(617) 496-7775
|
@matthewholman definitely you have 2-3 days to review this |
By default it's set to integrate_and_interpolate |
We haven't forgotten about this @KatKiker - We're working on getting in a data class for the config file parser that will make it easier to remove the global variables we have hard coded for ephemeris generation. Once that's done and this is rebased to include those upgrades, we'll likely be able to merge straight away. I think we'll be there by the end of the month. |
15ae572
to
9b316d6
Compare
Creates a config parameter to allow use of the rebound integrate method instead of assist's integrate_or_interpolate. create_ephemeris sets a global variable from the config which is checked in integrate_light_time. This seemed less messy than drilling the option down to the lower functions, but happy to do it that way as well if that's preferable. This circumvents the infinite bound errors we were seeing, and is helpful in fast moving or close-approach scenarios where the timestep is small.
Review Checklist for Source Code Changes