Skip to content
This repository has been archived by the owner on Oct 14, 2023. It is now read-only.

Properly vectorize propagators #922

Open
astrojuanlu opened this issue Apr 19, 2020 · 7 comments
Open

Properly vectorize propagators #922

astrojuanlu opened this issue Apr 19, 2020 · 7 comments

Comments

@astrojuanlu
Copy link
Member

At the moment, the "vectorization" step for most propagators happens outside of numba, harming performance. For example:

results = [farnocchia_fast(k, r0, v0, tof) for tof in tofs]
# TODO: Rewrite to avoid iterating twice
return (
[result[0] for result in results] * u.km,
[result[1] for result in results] * u.km / u.s,
)

This is related to #921 and perhaps should be tackled afterwards.

cc @Josvth

@astrojuanlu
Copy link
Member Author

Prospective contributors: consider looking at #921 first, then we can work on this.

@astrojuanlu
Copy link
Member Author

@andrea-carballo expressed interest in working on this! The steps should be:

  1. Move test_propagate_with_coe to a newly created tests/tests_core/test_propagation.py
  2. Move the functions in core/propagation/__init__.py to core/propagation/vallado.py, core/propagation/mikkola.py... as done with core/propagation/farnocchia.py and adapt tests and imports accordingly
  3. Write farnocchia_coe_tofs, mikkola_coe_tofs, markley_coe_tofs, pimienta_coe_tofs, gooding_coe_tofs, danby_coe_tofs doing this:
@jit
def propagator_tofs(k, ..., tofs, ...):
    nus = np.zeros_like(tofs)
    for tof in tofs:  # pseudocode
        # call corresponding propagator with single tof (time of flight) and store nu
    return nus

This doesn't fully fix the issue, but it's a first step.

@astrojuanlu
Copy link
Member Author

Postponing this to the next release.

@astrojuanlu astrojuanlu removed this from the 0.16 milestone Oct 17, 2021
@astrojuanlu astrojuanlu added this to the 0.17 milestone Dec 26, 2021
@astrojuanlu
Copy link
Member Author

Blocking this on #921.

@astrojuanlu astrojuanlu added the status:blocked Some other change is needed before working on this (see discussion) label Jan 16, 2022
@astrojuanlu
Copy link
Member Author

I think #921 will need a significant refactor, so I'm postponing this issue. At least will try to tackle #921 for the next release.

@astrojuanlu astrojuanlu removed this from the 0.17 milestone Jun 3, 2022
@astrojuanlu
Copy link
Member Author

#921 is about to be fixed. We can deal with this for the next release.

@astrojuanlu astrojuanlu removed the status:blocked Some other change is needed before working on this (see discussion) label Jul 7, 2022
@astrojuanlu astrojuanlu added this to the 0.18 milestone Jul 7, 2022
@astrojuanlu
Copy link
Member Author

Now propagators have a propagate_many method that still is iterating on the Python side rather than on the numba side:

results = np.array(
[farnocchia_rv_fast(k, *rv0, tof) for tof in tofs.to_value(u.s)]
)
return (
results[:, 0] << u.km,
results[:, 1] << (u.km / u.s),
)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Development

No branches or pull requests

2 participants