Skip to content
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

add PointingCalculator #2623

Draft
wants to merge 228 commits into
base: main
Choose a base branch
from
Draft

add PointingCalculator #2623

wants to merge 228 commits into from

Conversation

ctoennis
Copy link

I wanted to post this pull request here for the PointingCalculator.

I have not finished adding unit tests for the calculator and the code does not run as is.

One important thing to discuss is the method of looking up star locations. Currently the code uses astroquery, which would be a new dependency.

Otherwise this is still work in progress.

Tjark Miener and others added 30 commits April 29, 2024 08:51
added PlainExtractor based on numpy and scipy functions
restructured the stats containers
Remove StarVarianceExtractor since is functionality is featured in the existing Extractors
allow overlapping extraction sequences
renaming to chunk(s) and chunk_size and _shift

added test for chunk_shift and boundary case
@kosack
Copy link
Contributor

kosack commented Oct 3, 2024

One important thing to discuss is the method of looking up star locations. Currently the code uses astroquery, which would be a new dependency.

I think relying on accessing the internet to get star positions is not a good idea if these will run in large batch queues, as it is a point of failure and requires outside access. Instead, maybe we should think about just making one of configurable inputs be an astropy table (i.e. read the output of astroquery, but not requiring astroquery to be called internally).

I.e. if you have a config option star_catalog=<filename>, the user is responsible for creating it. We can give instructions how to do it using astroquery or another vo tool. Elsewhere in ctapipe, we already have something like that I think... using the bright star catalog. In principle if the catalog has UCDs in the columns, you don't even need to define the column names for such a table.

@ctoennis
Copy link
Author

ctoennis commented Oct 3, 2024

@kosack that sounds good. Since the cameras will often track specific objects we can pre-create such tables for those objects. The StarTracer thant needs to have the star positions can be just given a table of positions instead of using astroquery.

I will start out with some table for the crab nebula to have something for the unit tests.

@maxnoe
Copy link
Member

maxnoe commented Oct 3, 2024

@ctoennis the yale bright star catalog is already included in ctapipe as a static resource:
https://ctapipe.readthedocs.io/en/latest/api/ctapipe.utils.astro.get_bright_stars.html

It is complete to magnitude 6.5, which I think is enough for the pointing estimation, right?

If not, we can also include e.g. the hipparcos catalog down to a lower brightness.

To get a good idea of how you could use astroquery, have a look at this script I recently wrote to find stars suitable for PSF measurement:

https://github.com/maxnoe/find_psf_stars/blob/main/find_psf_stars.py

It queries the HIP catalog via astroquery to Vizier and downloads two files from the HIP webpage providing additionally common names for the stars where they are available.

@mexanick
Copy link
Contributor

mexanick commented Oct 3, 2024

In general vizier creates a cache, so we can make a setup with cached catalogs. However, we probably can get away with Yale bright star catalog too. @maxnoe you selection uses V-mag? We use B-mag for selection, so here we may need a bit of optimization on the range, but in general it should still work quite ok. @ctoennis please try this option

@maxnoe
Copy link
Member

maxnoe commented Oct 3, 2024

your selection uses V-mag? We use B-mag for selection

Yes, the current limit in the utility function is put on VMag, but you can simply change the function to have two arguments, max_vmag and max_bmag instead:

if magnitude_cut is not None:
catalog = catalog[catalog["Vmag"] < magnitude_cut]

@mexanick
Copy link
Contributor

mexanick commented Oct 3, 2024

I think we can try with the V-mag and see... 6.5 magnitude is close to the limit, we need to re-evaluate the star reconstruction performance to see whether we need fainter stars.

@maxnoe
Copy link
Member

maxnoe commented Oct 3, 2024

Ah, nevermind, the catalog export I did doesn't include Bmag, but you can also replace the fits file in the resources with an updated version that includes BMag if you need it.

@maxnoe
Copy link
Member

maxnoe commented Oct 3, 2024

After writing the psf stars script, I also realize that the function does not apply space motion, so stars with high proper motions will not be at the correct positions.

@ctoennis
Copy link
Author

ctoennis commented Oct 3, 2024

Ok, i'll switch to that tool.

@mexanick
Copy link
Contributor

mexanick commented Oct 3, 2024

ah, Vizier does it (and that's why it needs a periodic update of the cache)

@maxnoe
Copy link
Member

maxnoe commented Oct 3, 2024

ah, Vizier does it (and that's why it needs a periodic update of the cache)

I'd not select the computed values from Vizier, but use astropy, since then we can keep the cache for a long time and just have the correct proper motions applied. It's quite fast.

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

Successfully merging this pull request may close these issues.

5 participants