Add a configuration option to use integrate over integrate_or_interpolate

[KatKiker]

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

• Does pip install still work?
• Have you written a unit test for any new functions?
• Do all the units tests run successfully?
• Does Sorcha run successfully on a test set of input files/databases?
• Have you used black on the files you have updated to confirm python programming style guide enforcement?

[codecov]

Codecov Report

Attention: Patch coverage is 68.75000% with 5 lines in your changes missing coverage. Please review.

Project coverage is 81.08%. Comparing base (c2b800c) to head (15ae572).
Report is 24 commits behind head on main.

Files with missing lines	Patch %	Lines
src/sorcha/modules/PPConfigParser.py	42.85%	4 Missing ⚠️
src/sorcha/ephemeris/simulation_geometry.py	85.71%	1 Missing ⚠️

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.
📢 Have feedback on the report? Share it here.

[bernardinelli]

This is great!!! One question before approving: which one is set by default? Integrate_or_interpolate or just integrate?

[matthewholman]

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

[mschwamb]

@matthewholman definitely you have 2-3 days to review this

[KatKiker]

This is great!!! One question before approving: which one is set by default? Integrate_or_interpolate or just integrate?

By default it's set to integrate_and_interpolate

[mschwamb]

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.

[mschwamb]

@KatKiker we've made some updates that now have all of the ephemeris generation properties in the config file with variables in a config file data class. Can you rebase to follow the architecture in just merged PR #1077 and then we'll be able to get this included

[mschwamb]

@KatKiker and @akoumjian it may be closer to build from a clean version and close this PR and submit a new one. We've got one more PR that will do some changes to ephemeris generator files #1094 so you might want to wait until that's merged. I expect that will be Weds. We might have some bandwidth to implement the changes into the current version of the main branch later this week/early next week. The aim is to submit the code/papers next week.

[akoumjian]

Thanks, Meg. We will keep our eyes open and rebase when possible.

[mschwamb]

PR #1094 is merged so feel free to rebase - there should be no more big changes to ephemeris generation. All the big tickets are closed for v1.0 🤞

[matthewholman]

Sorry to be slow in responding. It seems like this is an issue with ASSIST rather than with Sorcha.

[akoumjian]

@matthewholman @mschwamb Just re-completed the checklist after rebasing and it appears to pass all linting and tests.

I think this is a very useful option for us, even if the interpolate function in ASSIST addresses the infinite bound errors, and would appreciate it getting merged in. We use assist integrate in separate processes in our research and it is helpful to be running identical integrations for particular time steps.