Optional integrate

[KatKiker]

Fixes # .

Adds a config parameter ar_use_integrate that uses the rebound integrate method instead of assist's integrate_and_interpolate. This is important for accuracy for close approaches and fast movers.

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?

[akoumjian]

Unfortunately because of the way they structured this, it looks like it requires drilling down the option to the lower level function. I would recommend instead of extracting use_integrate and pushing it down, I would update all those functions to take the config object instead. Then inside integrate_light_time, look for the ar_use_integrate option and use it.

At least then it's not a single option being passed down but a config. I would put in your final PR that they could also consider creating a global CONFIG which could be accessed without passing it down, or moving to a class based structure so it can be referenced via self.

[KatKiker]

Unfortunately because of the way they structured this, it looks like it requires drilling down the option to the lower level function. I would recommend instead of extracting use_integrate and pushing it down, I would update all those functions to take the config object instead. Then inside integrate_light_time, look for the ar_use_integrate option and use it.

At least then it's not a single option being passed down but a config. I would put in your final PR that they could also consider creating a global CONFIG which could be accessed without passing it down, or moving to a class based structure so it can be referenced via self.

Can do! I wasn't sure which would be preferable to them, passing through the entire config or extracting just the value that was needed.

[KatKiker]

Unfortunately because of the way they structured this, it looks like it requires drilling down the option to the lower level function. I would recommend instead of extracting use_integrate and pushing it down, I would update all those functions to take the config object instead. Then inside integrate_light_time, look for the ar_use_integrate option and use it.

At least then it's not a single option being passed down but a config. I would put in your final PR that they could also consider creating a global CONFIG which could be accessed without passing it down, or moving to a class based structure so it can be referenced via self.

The benefit of passing just the use_integrate value is that it can be set to a default, meaning none of the tests or other instances of the function being called need to be updated. I'm not sure the same can be said for passing config. (That is to say- I feel like setting config as Optional (default None presumably) could be confusing and possibly messy, but maybe that's just what needs to happen. Alternatively, if not optional, then it would be a breaking change)

[akoumjian]

A less than ideal option would be that the config value sets an os.environ value and we read that environment variable in during the function that calls assist.

That might be the least instrusive...