Skip to content

DOC: avoid iteration for light travel time of lots of objects #7755

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

Open
StuartLittlefair opened this issue Aug 22, 2018 · 14 comments
Open

DOC: avoid iteration for light travel time of lots of objects #7755

StuartLittlefair opened this issue Aug 22, 2018 · 14 comments
Labels

Comments

@StuartLittlefair
Copy link
Contributor

StuartLittlefair commented Aug 22, 2018

In our user survey, @stargaser mentioned they had issues at ZTF when calculating light travel times for tens of thousands of sources in a degree patch of the sky. It might be worth a section in the performance area of the Time documentation comparing run times of the following approaches:

import numpy as np
import astropy.coordinates as coord
import astropy.units as u
from astropy.time import Time

ra = np.random.normal(0.0, 1.0, 50000)
dec = np.random.normal(0.0, 1.0, 50000)

coos = coord.SkyCoord(ra, dec, unit=u.deg)
observatory = coord.EarthLocation.of_site('lapalma')

%time time.light_travel_time(coos, location=observatory)
CPU times: user 56 ms, sys: 3.21 ms, total: 59.2 ms
Wall time: 58.2 ms
<TimeDelta object: scale='tdb' format='jd' value=[ 0.00497521  0.00495056  0.00497048 ...,  0.00499594  0.0049838
  0.00502038]>

%time ltts = [time.light_travel_time(coo, location=coos.location) for coo in coos]
CPU times: user 16min 45s, sys: 5.08 s, total: 16min 50s
Wall time: 16min 58s
@StuartLittlefair StuartLittlefair added Docs time Effort-low Package-novice good first issue Issues that are well-suited for new contributors labels Aug 22, 2018
@StuartLittlefair
Copy link
Contributor Author

For user cases where there are thousands of times for each source, broadcasting can be used:

times = Time.now() + np.linspace(0, 3, 1000)*u.day
%time ltts = times.light_travel_time(coos[:, np.newaxis], location=observatory)
CPU times: user 13.8 s, sys: 13 s, total: 26.8 s
Wall time: 27.9 s

@shilpi1958
Copy link
Contributor

Hi, I am shilpi. I am new to opensource and would like to contribute.
How can i start to work on this issue?

@pllim
Copy link
Member

pllim commented Oct 26, 2018

@shilpi1958 , I don't see why not. http://www.astropy.org/contribute.html

@astrofrog
Copy link
Member

I think this has already been started in #7893

@kakirastern
Copy link
Contributor

I have just taken over this issue from @SG004 with his permission following his attempts at Issue #7893 without attaining some resolution. Will open another PR to try to follow in his footsteps...

@kakirastern
Copy link
Contributor

kakirastern commented Feb 21, 2019

So I can reproduce similar results/effects in comparative performance after fixing some bugs using the following:

>>> import numpy as np
>>> import astropy.coordinates as coord
>>> import astropy.units as u
>>> from astropy.time import Time

>>> ra = np.random.normal(0.0, 1.0, 5000)
>>> dec = np.random.normal(0.0, 1.0, 5000)

>>> coos = coord.SkyCoord(ra, dec, unit=u.deg)
>>> observatory = coord.EarthLocation.of_site('lapalma')

>>> %time Time.light_travel_time(Time.now(), coos, location=observatory)
WARNING: failed to download http://maia.usno.navy.mil/ser7/finals2000A.all and http://toshi.nofs.navy.mil/ser7/finals2000A.all, using local IERS-B: <urlopen error timed out>;<urlopen error timed out> [astropy.utils.iers.iers]
WARNING: Tried to get polar motions for times after IERS data is valid. Defaulting to polar motion from the 50-yr mean for those. This may affect precision at the 10s of arcsec level [astropy.coordinates.builtin_frames.utils]
WARNING: (some) times are outside of range covered by IERS table. Assuming UT1-UTC=0 for coordinate transformations. [astropy.coordinates.builtin_frames.utils]
CPU times: user 42.5 ms, sys: 16.6 ms, total: 59.2 ms
Wall time: 40.1 s
<TimeDelta object: scale='tdb' format='jd' value=[-0.00501821 -0.00499115 -0.00501107 ... -0.00514924 -0.00506957
 -0.00508886]>

>>>%time ltts = [Time.light_travel_time(Time.now(), coo, location=observatory) for coo in coos]
WARNING: failed to download http://maia.usno.navy.mil/ser7/finals2000A.all and http://toshi.nofs.navy.mil/ser7/finals2000A.all, using local IERS-B: <urlopen error timed out>;<urlopen error timed out> [astropy.utils.iers.iers]
WARNING: Tried to get polar motions for times after IERS data is valid. Defaulting to polar motion from the 50-yr mean for those. This may affect precision at the 10s of arcsec level [astropy.coordinates.builtin_frames.utils]
WARNING: (some) times are outside of range covered by IERS table. Assuming UT1-UTC=0 for coordinate transformations. [astropy.coordinates.builtin_frames.utils]
... (Timed out. Takes too long on personal laptop, will update if available.) 

and with broadcasting

>>> times = Time.now() + np.linspace(0, 3, 1000)*u.day
>>> %time ltts = times.light_travel_time(coos[:, np.newaxis], location=observatory)
WARNING: failed to download http://maia.usno.navy.mil/ser7/finals2000A.all and http://toshi.nofs.navy.mil/ser7/finals2000A.all, using local IERS-B: <urlopen error timed out>;<urlopen error timed out> [astropy.utils.iers.iers]
WARNING: Tried to get polar motions for times after IERS data is valid. Defaulting to polar motion from the 50-yr mean for those. This may affect precision at the 10s of arcsec level [astropy.coordinates.builtin_frames.utils]
WARNING: (some) times are outside of range covered by IERS table. Assuming UT1-UTC=0 for coordinate transformations. [astropy.coordinates.builtin_frames.utils]
CPU times: user 1.66 s, sys: 766 ms, total: 2.42 s
Wall time: 43.4 s

So on the surface everything seems to be in order. Using iteration definitely aversely affect the performance of the system when invoking light_travel_time. I reckon I should compare the three cases in the relevant documentation in terms of relative lengths of time elapsed, and issue some cautionary warning about coding approach where appropriate.

@kakirastern
Copy link
Contributor

Discovered @SG004's errors in PR #7893 has been due to some bugs corrected as the code blocks stated in the previous comment, which is a minor problem. Will consult SG004 again about whether he would like to continue with his previous work. So now all is good.

@KING-SID
Copy link

Hey is this issue still open? I tried to follow the comment thread but I couldnt really tell. Let me know if it open as I would like to work on this!

@bsipocz
Copy link
Member

bsipocz commented Feb 23, 2019

@KING-SID - I think @kakirastern and @SG004 are on top of this, see the conversation in their PR.

@kakirastern
Copy link
Contributor

Hi @KING-SID, I am still awaiting @SG004's confirmation to see if he still wants to follow up on his previous work and continue until his PR (or a new one, depends) is merged/closed. If he decides he doesn't want to work on it anymore then I will take over with a new PR. So as @bsipocz has stated, we are on top of this. Thanks for your interest, however.

@kakirastern
Copy link
Contributor

@SG004 will continue work on PR #7893 to update the relevant documentation.

@YasPHP
Copy link

YasPHP commented Sep 1, 2020

Hey previous comment thread, is this issue still open? haha I couldn't tell seeing as it's still open and a "good first issue"

@kakirastern
Copy link
Contributor

I think it is still open, as @SG004's PR #8461 dealing with the issue has been closed by a bot.

@YasPHP
Copy link

YasPHP commented Sep 1, 2020

Gotcha, ok thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants