-
Notifications
You must be signed in to change notification settings - Fork 269
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
Proper motion #2625
base: main
Are you sure you want to change the base?
Proper motion #2625
Changes from all commits
2c4f4cf
3be8ede
704f7a0
12c49f0
8de6331
0ce70b7
0ca8561
4f28af2
ced74cc
80dfc87
0d6c5a4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
Add ''get_bright_stars_with_motion'' to ctapipe.utils.astro | ||
this function allows to look up the position of stars | ||
around a specified location including proper motion. | ||
different catalogues can be used. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,6 +60,7 @@ tests = [ | |
"pytest-xdist", | ||
"pytest_astropy_header", | ||
"tomli", | ||
"astroquery", | ||
] | ||
|
||
docs = [ | ||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -4,63 +4,231 @@ | |||
not provided by external packages and/or to satisfy particular needs of | ||||
usage within ctapipe. | ||||
""" | ||||
from astropy import units as u | ||||
|
||||
import logging | ||||
from enum import Enum | ||||
from pathlib import Path | ||||
|
||||
import astropy.units as u | ||||
from astropy.coordinates import Angle, SkyCoord | ||||
from astropy.table import Table | ||||
from astropy.time import Time | ||||
|
||||
log = logging.getLogger("main") | ||||
|
||||
__all__ = ["get_bright_stars"] | ||||
|
||||
|
||||
def get_bright_stars(pointing=None, radius=None, magnitude_cut=None): | ||||
CACHE_FILE = Path("~/.psf_stars.ecsv").expanduser() | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs to be changed. As discussed in the comments, we want to have the catalogs as resource files in the package and only have the code to create them for developers to update those files if needed. So this should not be needed at all. if we need it, it should be using this function here to obtain an appropriate cache path: ctapipe/src/ctapipe/utils/download.py Line 69 in 40faf69
instead of just storing a hidden file in the user's home |
||||
|
||||
|
||||
class StarCatalogues(Enum): | ||||
Yale = "V/50/catalog" # Yale bright star catalogue | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Change the comment to be above and have the form |
||||
Hippoarcos = "I/239/hip_main" # hipparcos catalogue | ||||
|
||||
|
||||
def select_stars(stars, pointing=None, radius=None, Bmag_cut=None, Vmag_cut=None): | ||||
""" | ||||
utility function to cut stars based on magnitude and/or location | ||||
|
||||
Parameters | ||||
---------- | ||||
stars: astropy table | ||||
Table of stars, including magnitude and coordinates | ||||
pointing: astropy Skycoord | ||||
pointing direction in the sky (if none is given, full sky is returned) | ||||
radius: astropy angular units | ||||
Radius of the sky region around pointing position. Default: full sky | ||||
Vmag_cut: float | ||||
Return only stars above a given apparent magnitude. Default: None (all entries) | ||||
Bmag_cut: float | ||||
Return only stars above a given absolute magnitude. Default: None (all entries) | ||||
|
||||
Returns | ||||
------- | ||||
Astropy table: | ||||
List of all stars after cuts with same keys as the input table stars | ||||
""" | ||||
Get an astropy table of bright stars. | ||||
|
||||
This function is using the Yale bright star catalog, available through ctapipe | ||||
data downloads. | ||||
if Bmag_cut is not None and "Bmag" in stars.keys(): | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are Why was it needed to add the "T" versions? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. some catalogs only have the T versions available There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But we only deal with two here, which to my knowledge have Bmag and Vmag There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. here is the link to the hippoarcos catalogue: https://vizier.cds.unistra.fr/viz-bin/VizieR-3?-source=I/239/hip_main&-out.max=50&-out.form=HTML%20Table&-out.add=_r&-out.add=_RAJ,_DEJ&-sort=_r&-oc.form=sexa It has Vmag, but no Bmag. It has BTmag and VTmag available. The Yale catalogue is here: Here it's only Vmag. the Nomad catalogue has both Bmag and Vmag: https://vizier.cds.unistra.fr/viz-bin/VizieR-2 If it you want to not have the T versions i can remove it and add error messages when you try to use Bmag with the Yale and Hippoarcos catalogues. |
||||
stars = stars[stars["Bmag"] < Bmag_cut] | ||||
elif Bmag_cut is not None and "BTmag" in stars.keys(): | ||||
stars = stars[stars["BTmag"] < Bmag_cut] | ||||
if Vmag_cut is not None and "Vmag" in stars.keys(): | ||||
stars = stars[stars["Vmag"] < Vmag_cut] | ||||
elif Vmag_cut is not None and "VTmag" in stars.keys(): | ||||
stars = stars[stars["VTmag"] < Vmag_cut] | ||||
|
||||
The included Yale bright star catalog contains all 9096 stars, excluding the | ||||
Nova objects present in the original catalog and is complete down to magnitude | ||||
~6.5, while the faintest included star has mag=7.96. :cite:p:`bright-star-catalog` | ||||
if radius is not None: | ||||
if pointing is None: | ||||
raise ValueError( | ||||
"Sky pointing, pointing=SkyCoord(), must be " | ||||
"provided if radius is given." | ||||
) | ||||
separations = stars["ra_dec"].separation(pointing) | ||||
stars["separation"] = separations | ||||
stars = stars[separations < radius] | ||||
|
||||
return stars | ||||
|
||||
|
||||
def get_star_catalog(catalog, filename): | ||||
""" | ||||
Utility function to download a star catalog for the get_bright_stars function | ||||
|
||||
Parameters | ||||
---------- | ||||
catalog: string | ||||
Name of the catalog to be used. Usable names are found in the Enum StarCatalogues. Default: Yale | ||||
filename: string | ||||
Name and location of the catalog file to be produced | ||||
""" | ||||
from astroquery.vizier import Vizier | ||||
|
||||
vizier = Vizier( | ||||
catalog=StarCatalogues[catalog].value, | ||||
columns=[ | ||||
"recno", | ||||
"RAJ2000", | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please remove the computed "RAJ2000" / "DEJ2000" values. These are computed by vizier on the fly and are redundant with the original coordinates and the proper motions. We anyway need to apply proper motion, so it doesn't matter whether we start from the catalog epoch or j2000 |
||||
"DEJ2000", | ||||
"RAICRS", | ||||
"DEICRS", | ||||
"pmRA", | ||||
"pmDE", | ||||
"Vmag", | ||||
"Bmag", | ||||
"BTmag", | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why also the T versions? |
||||
"VTmag", | ||||
], | ||||
row_limit=1000000, | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be a keyword argument. Or at least there should be a warning if |
||||
) | ||||
|
||||
stars = vizier.query_constraints(Vmag="0.0..10.0")[0] | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why limit this to magnitude 0.0? There are a handful of brighters stars. The limit to magnitude 0 is a rule of LST for psf stars, not applicable here. The constraints should be an option to this function as well. |
||||
|
||||
if "Bmag" not in stars.keys(): | ||||
log.exception("The chosen catalog does not have Bmag data") | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is wrong usage of This should be an actual exception, so throw a |
||||
if "Vmag" not in stars.keys(): | ||||
log.exception("The chosen catalog does not have Vmag data") | ||||
stars.meta["Catalog"] = StarCatalogues[catalog].value | ||||
|
||||
stars.write(CACHE_FILE, overwrite=True) | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. THis should use the (at the moment unused) But I would completely remove the |
||||
|
||||
|
||||
def get_bright_stars( | ||||
pointing=None, radius=None, Bmag_cut=None, Vmag_cut=None, catalog="Yale" | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||
): # max_magnitude): | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove commented part There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For backwards compatibilty, we could think about keeping the |
||||
""" | ||||
Get an astropy table of bright stars from a VizieR catalog | ||||
|
||||
You can browse the catalogs at https://vizier.cds.unistra.fr/viz-bin/VizieR | ||||
|
||||
Parameters | ||||
---------- | ||||
pointing: astropy Skycoord | ||||
pointing direction in the sky (if none is given, full sky is returned) | ||||
radius: astropy angular units | ||||
Radius of the sky region around pointing position. Default: full sky | ||||
magnitude_cut: float | ||||
Return only stars above a given magnitude. Default: None (all entries) | ||||
Vmag_cut: float | ||||
Return only stars above a given apparent magnitude. Default: None (all entries) | ||||
Bmag_cut: float | ||||
Return only stars above a given absolute magnitude. Default: None (all entries) | ||||
catalog: string | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Create an enum with human-readable catalog names. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i made an enum with 3 catalogues. let me know if i should add any specific catalog that would be useful |
||||
Name of the catalog to be used. Usable names are found in the Enum StarCatalogues. Default: Yale | ||||
|
||||
Returns | ||||
------- | ||||
Astropy table: | ||||
List of all stars after cuts with names, catalog numbers, magnitudes, | ||||
and coordinates | ||||
""" | ||||
from ctapipe.utils import get_table_dataset | ||||
|
||||
catalog = get_table_dataset("yale_bright_star_catalog5", role="bright star catalog") | ||||
stars = None | ||||
|
||||
starpositions = SkyCoord( | ||||
ra=Angle(catalog["RAJ2000"], unit=u.deg), | ||||
dec=Angle(catalog["DEJ2000"], unit=u.deg), | ||||
frame="icrs", | ||||
copy=False, | ||||
) | ||||
catalog["ra_dec"] = starpositions | ||||
if CACHE_FILE.exists(): | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should use the importlib resources system to load the resources file for the specified catalog |
||||
log.info("Loading stars from cached table") | ||||
try: | ||||
stars = Table.read(CACHE_FILE) | ||||
if stars.meta["Catalog"] == StarCatalogues[catalog].value: | ||||
if Bmag_cut is not None and "Bmag_cut" in stars.meta.keys(): | ||||
if stars.meta["Bmag_cut"] >= Bmag_cut: | ||||
log.debug(f"Loaded table is valid for { Bmag_cut= }") | ||||
else: | ||||
log.debug( | ||||
"Loaded cache table has smaller magnitude_cut, reloading" | ||||
) | ||||
stars = None | ||||
if Vmag_cut is not None and "Vmag_cut" in stars.meta.keys(): | ||||
if stars.meta["Vmag_cut"] >= Vmag_cut: | ||||
log.debug(f"Loaded table is valid for {Vmag_cut= }") | ||||
else: | ||||
log.debug( | ||||
"Loaded cache table has smaller magnitude_cut, reloading" | ||||
) | ||||
stars = None | ||||
else: | ||||
stars = None | ||||
except Exception: | ||||
log.exception("Cache file exists but reading failed. Recreating") | ||||
|
||||
if magnitude_cut is not None: | ||||
catalog = catalog[catalog["Vmag"] < magnitude_cut] | ||||
if stars is None: | ||||
from astroquery.vizier import Vizier | ||||
|
||||
if radius is not None: | ||||
if pointing is None: | ||||
raise ValueError( | ||||
"Sky pointing, pointing=SkyCoord(), must be " | ||||
"provided if radius is given." | ||||
) | ||||
separations = catalog["ra_dec"].separation(pointing) | ||||
catalog["separation"] = separations | ||||
catalog = catalog[separations < radius] | ||||
log.info("Querying Vizier for bright stars catalog") | ||||
# query vizier for stars with 0 <= Vmag <= max_magnitude | ||||
vizier = Vizier( | ||||
catalog=StarCatalogues[catalog].value, | ||||
columns=[ | ||||
"recno", | ||||
"RAJ2000", | ||||
"DEJ2000", | ||||
"RAICRS", | ||||
"DEICRS", | ||||
"pmRA", | ||||
"pmDE", | ||||
"Vmag", | ||||
"Bmag", | ||||
"BTmag", | ||||
"VTmag", | ||||
], | ||||
row_limit=1000000, | ||||
) | ||||
|
||||
stars = vizier.query_constraints(Vmag="0.0..10.0")[0] | ||||
if "Bmag" in stars.keys(): | ||||
if Bmag_cut is not None: | ||||
stars.meta["Bmag_cut"] = Bmag_cut | ||||
elif Bmag_cut is not None: | ||||
log.exception("The chosen catalog does not have Bmag data") | ||||
if "Vmag" in stars.keys(): | ||||
if Vmag_cut is not None: | ||||
stars.meta["Vmag_cut"] = Vmag_cut | ||||
elif Vmag_cut is not None: | ||||
log.exception("The chosen catalog does not have Vmag data") | ||||
stars.meta["Catalog"] = StarCatalogues[catalog].value | ||||
|
||||
catalog.remove_columns(["RAJ2000", "DEJ2000"]) | ||||
stars.write(CACHE_FILE, overwrite=True) | ||||
|
||||
if "RAJ2000" in stars.keys(): | ||||
stars["ra_dec"] = SkyCoord( | ||||
ra=Angle(stars["RAJ2000"], unit=u.deg), | ||||
dec=Angle(stars["DEJ2000"], unit=u.deg), | ||||
pm_ra_cosdec=stars["pmRA"].quantity, # yes, pmRA is already pm_ra_cosdec | ||||
pm_dec=stars["pmDE"].quantity, | ||||
frame="icrs", | ||||
obstime=Time("J2000.0"), | ||||
) | ||||
elif "RAICRS" in stars.keys(): | ||||
stars["ra_dec"] = SkyCoord( | ||||
ra=Angle(stars["RAICRS"], unit=u.deg), | ||||
dec=Angle(stars["DEICRS"], unit=u.deg), | ||||
pm_ra_cosdec=stars["pmRA"].quantity, # yes, pmRA is already pm_ra_cosdec | ||||
pm_dec=stars["pmDE"].quantity, | ||||
frame="icrs", | ||||
obstime=Time("J1991.25"), | ||||
) | ||||
|
||||
stars = select_stars( | ||||
stars, pointing=pointing, radius=radius, Bmag_cut=Bmag_cut, Vmag_cut=Vmag_cut | ||||
) | ||||
|
||||
return catalog | ||||
return stars |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,19 +3,46 @@ | |
This module contains the utils.astro unit tests | ||
""" | ||
from astropy import units as u | ||
from astropy.coordinates import SkyCoord | ||
from astropy.coordinates import Angle, SkyCoord | ||
from astropy.time import Time | ||
|
||
from ..astro import get_bright_stars | ||
|
||
|
||
def test_get_bright_stars(): | ||
""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you add more catalogs, it makes sense to test all of them. Also, it would be nice to test a but more details, or add more comments about the expected outcome of the query. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oka, i will get to that part tomorrow morning |
||
unit test for utils.astro.get_bright_stars(). Tests that only Zeta Tau is | ||
returned close to the Crab Nebula as object brighter than mag=3.5. | ||
unit test for utils.astro.get_bright_stars_with_motion(). | ||
""" | ||
pointing = SkyCoord(ra=83.275 * u.deg, dec=21.791 * u.deg, frame="icrs") | ||
# TODO add tests for all catalogues, specifically by trying to find some particular bright star, also test that motion is properly included | ||
eta_tau = SkyCoord( | ||
ra=Angle("03 47 29.1", unit=u.deg), | ||
dec=Angle("+24 06 18", unit=u.deg), | ||
frame="icrs", | ||
) | ||
obstime = Time("2020-01-01") | ||
|
||
table = get_bright_stars(pointing, radius=2.0 * u.deg, magnitude_cut=3.5) | ||
# lets find 25 Eta Tau | ||
|
||
table = get_bright_stars(pointing=eta_tau, radius=1.0 * u.deg, Vmag_cut=3.5) | ||
|
||
assert len(table) == 1 # this looks if 25 Eita Tau was found | ||
# check that the star moves | ||
assert table[0]["ra_dec"].apply_space_motion(obstime).ra != eta_tau.ra | ||
assert table[0]["ra_dec"].apply_space_motion(obstime).dec != eta_tau.dec | ||
|
||
# now test the other catalog. First get object 766 from the catalog | ||
|
||
HIP_star = SkyCoord( | ||
ra=Angle("00 08 23.2585712", unit=u.deg), | ||
dec=Angle("+29 05 25.555166", unit=u.deg), | ||
frame="icrs", | ||
) | ||
|
||
table = get_bright_stars( | ||
pointing=HIP_star, radius=1.0 * u.deg, Vmag_cut=3.5, catalog="Hippoarcos" | ||
) | ||
|
||
assert len(table) == 1 | ||
assert table[0]["Name"] == "123Zet Tau" | ||
# now check that stars move | ||
assert table[0]["ra_dec"].apply_space_motion(obstime).ra != eta_tau.ra | ||
assert table[0]["ra_dec"].apply_space_motion(obstime).dec != eta_tau.dec |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update the changelog once we have this finalized