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

Avoid iterating results twice in propagation.py #1286

Merged
merged 1 commit into from
Jul 23, 2021

Conversation

Yash-10
Copy link
Member

@Yash-10 Yash-10 commented Jul 21, 2021

This tries to solve a "TODO" in the propagation module where the variable results was iterated twice.

The resulting improvement is ~1s for propagation for 50 seconds.

Thanks!


Re-triggering still shows the tests as canceled. Maybe some remote issue. Not now

@Yash-10 Yash-10 force-pushed the Slight-propagation-optimization branch 2 times, most recently from 167c625 to 73085d2 Compare July 21, 2021 18:36
S

reformat

smal fix

SS
@Yash-10 Yash-10 force-pushed the Slight-propagation-optimization branch from 73085d2 to f39c1ed Compare July 21, 2021 18:49
@Yash-10 Yash-10 closed this Jul 21, 2021
@Yash-10 Yash-10 reopened this Jul 21, 2021
@Yash-10 Yash-10 closed this Jul 22, 2021
@Yash-10 Yash-10 reopened this Jul 22, 2021
@codecov
Copy link

codecov bot commented Jul 22, 2021

Codecov Report

Merging #1286 (f39c1ed) into main (cec10f5) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1286   +/-   ##
=======================================
  Coverage   91.99%   91.99%           
=======================================
  Files          81       81           
  Lines        4248     4248           
  Branches      361      354    -7     
=======================================
  Hits         3908     3908           
  Misses        258      258           
  Partials       82       82           
Impacted Files Coverage Δ
src/poliastro/twobody/propagation.py 91.48% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cec10f5...f39c1ed. Read the comment docs.

Copy link
Member

@jorgepiloto jorgepiloto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me @Yash-10, no more comments by my side. Let us wait for @astrojuanlu approval too 🚀

Copy link
Member

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay, thanks @Yash-10 !

@astrojuanlu astrojuanlu merged commit 2bf5ce0 into poliastro:main Jul 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants