ligo.skymap merge requestshttps://git.ligo.org/lscsoft/ligo.skymap/-/merge_requests2024-03-27T19:52:26Zhttps://git.ligo.org/lscsoft/ligo.skymap/-/merge_requests/360Draft: Upload pytest-mpl images as junit attachments2024-03-27T19:52:26ZLeo P. SingerDraft: Upload pytest-mpl images as junit attachmentshttps://git.ligo.org/lscsoft/ligo.skymap/-/merge_requests/359Add unit tests for Python 3.122024-03-25T18:54:43ZLeo P. SingerAdd unit tests for Python 3.12https://git.ligo.org/lscsoft/ligo.skymap/-/merge_requests/347Draft: Proposal : Simplified API2024-02-22T08:06:46ZThomas SainratDraft: Proposal : Simplified API## Disclaimer
First and foremost, this MR is meant as a proof of concept ; it is incomplete and SHOULD NOT be merged in the current state. It aims to propose a simplified API for the package, and currently adds a file (partially) implem...## Disclaimer
First and foremost, this MR is meant as a proof of concept ; it is incomplete and SHOULD NOT be merged in the current state. It aims to propose a simplified API for the package, and currently adds a file (partially) implementing this API ; it does not induce any breaking change this way, but the ultimate goal would be to actually modify the rest of the package on the long-term. This would likely result breaking changes (although there may be ways to keep the current behavior working), meaning this would likely lead to a version 2.0 of the package, and require strategies to allow users to adapt their code to the new version.
## Rationale
In its current state, the user of the `ligo.skymap` package is currently confronted to several different types of skymap representations:
- the regular HealPIX ("fixed-ordering") format, typically in the form of a numpy array (but can also be an astropy Table) ; it can have two different orderings, NESTED and RING.
- the newer MOC format, typically as an astropy Table or a numpy structured array. As a user of the package who have been working with the different functions (plotting, reading and writing, postprocess), I have been confused multiple times on which representation should be used ; I have several times been mistaken between the NESTED and RING types (which are only distinguished by metadata), and have spent some time finding how to convert between MOC and fixed-ordering formats, which is not well documented currently. This is not aided by the fact that most functions will only accept one of the formats, which is not consistent across all functions (for instance, a non-trivial operation is to generate a skymap with `bayestar.localize` (returns MOC format) and apply `find_greedy_credible_levels` (takes in the fixed-ordering format)).
## Proposal
This PR introduces a new `Skymap` class, which can be instantiated from any of the aforementioned skymap representations, and provide convenience utilities to convert between the different formats. The changes to the API would require that any function currently receiving a skymap (in either format) should now take in the new Skymap object. This is all implemented in the `ligo.skymap.skymap` module (for a lack of a better name) ; it contains the `Skymap` class, some of the functions from other modules wrapped (almost all of `postprocess`, `bayestar.localize`, the read and write function from `moc`), as well as a wrapper around `ligo.skymap.plot` providing similar functionality (adding the relevant matplotlib projections). Examples of usage can be found in the main module `tests` folder.
## Possible path forward
This MR is mostly meant as a proposal ; it does not require an immediate acceptation, and I am fully opened to other alternatives. It has not been extensively tested aside from the included `pytest` files. In the case where we want to move forward with this, there are several possible options :
- keep the same format as the MR, with a separate file containing the simplified API, and advertising it to users ; this requires wrapping the currently unwrapped functions, which is a small amount of work, as well as documenting it.
- proceed with modifying the current API in a non-backward compatible way. This should trigger a 2.0 version of the package ; existing users would need to restrict themselves to the 1.x versions to keep using the old API. A period where both the 2.x and 1.x versions would be updated might be necessary.
- find a way to keep backward compatibility, by keeping the current API intact while also allowing for the new Skymap object to be used. This requires more work than the other options, and would introduce more code into the package, but would keep existing codes functioning ; it may also be a transition period with the previous option, raising warnings for users to use the new API. EDIT : I may have some ideas on how this could be done with a limited amount of code, so this may be easier than I thought.https://git.ligo.org/lscsoft/ligo.skymap/-/merge_requests/346Draft: Add inclination angle samples to the output skymap2024-03-14T14:33:19ZJacopo TissinoDraft: Add inclination angle samples to the output skymapThis MR accomplishes the following tasks:
* [x] Saving the inclination angle samples as part of the low-level functions `bayestar_sky_map_toa_phoa_snr` and `bayestar_sky_map_toa_phoa_snr_pixel`
* [x] Introducing the `output-inclination`...This MR accomplishes the following tasks:
* [x] Saving the inclination angle samples as part of the low-level functions `bayestar_sky_map_toa_phoa_snr` and `bayestar_sky_map_toa_phoa_snr_pixel`
* [x] Introducing the `output-inclination` option to `bayestar-localize-lvalerts` and `bayestar-localize-coincs`
* [ ] At the moment, this is implemented in a very simplistic way: the inclination angle samples are computed for either value of the option, they are just not included in the final skymap if the option is turned off. It may be worth it to propagate the flag down to the C code for efficiency.
* [x] Adapting the `rasterize` function so that it also correctly applies to the `*_SAMPLES` columns of the skymap
* [ ] This often results in very heavy output files. Should we change the `rasterize` default to only `'PROBDENSITY'` or `['PROBDENSITY', 'DISTMU', 'DISTSIGMA', DISTNORM']` being rasterized?
* [x] Adding some unit tests for the skymap as part of `test_bayestar.py`
* Checking for the correct normalizations of both the samples and the probability density
* Checking for the correct columns being present when the `output_inclination` option to `localize` is either on or off
* Refactoring the tests using module-scoped fixtures so that the single-detector skymap is only computed once
The new columns being added to the skymap are named `['PROBDENSITY_SAMPLES', 'DISTMU_SAMPLES', 'DISTSIGMA_SAMPLES', DISTNORM_SAMPLES']`.
Each of them contains 10 entries, corresponding to 10 equally-spaced inclination angles between $0$ and $\pi$.
The quadrature scheme for the inclination integral is also switched here to Clenshaw-Curtis quadrature (whereas it was Gauss-Legendre quadrature before).
The work within this MR is heavily based on a previous implementation by @leo-singer.
Other minor points:
- [ ] should we include some utility functions within the `postprocess` module for common tasks such as computing skymaps conditioned on a given inclination angle?
- [ ] the settings for `flake8` within `tox.ini` and the CI seem to differ - the maximum line length is 100 in the former, 79 in the latter --- should `tox.ini` be updated?
- [ ] should I document this change as part of release 1.1.3 or 1.2.0? To me it seems like a significant enough difference in interface to warrant a new minor release number.https://git.ligo.org/lscsoft/ligo.skymap/-/merge_requests/339ligo-skymap-plot now uses multi-resolution HEALPix natively2023-10-31T15:57:39ZLeo P. Singerligo-skymap-plot now uses multi-resolution HEALPix nativelyEnhancements to the command-line tool `ligo-skymap-plot`:
- Read in and reproject sky maps in multi-resolution format. This
significantly decreases memory consumption when plotting
high-resolution sky maps.
- The algorithm that cal...Enhancements to the command-line tool `ligo-skymap-plot`:
- Read in and reproject sky maps in multi-resolution format. This
significantly decreases memory consumption when plotting
high-resolution sky maps.
- The algorithm that calculates credible areas printed in the
plot legend now matches the algorithm in
`ligo.skymap.postprocess.crossmatch`: it does linear
interpolation of area when the credible level falls between
pixels.https://git.ligo.org/lscsoft/ligo.skymap/-/merge_requests/289Draft: Use signed integer for loop variables2023-06-30T17:59:07ZLeo P. SingerDraft: Use signed integer for loop variablesUnsigned integer for loop variables prevent loop unrolling and
may come with a performance penalty.
This is a well-known pitfall in C related to undefined behavior.
See, for example, https://stackoverflow.com/questions/52862923/signed-o...Unsigned integer for loop variables prevent loop unrolling and
may come with a performance penalty.
This is a well-known pitfall in C related to undefined behavior.
See, for example, https://stackoverflow.com/questions/52862923/signed-or-unsigned-loop-counterhttps://git.ligo.org/lscsoft/ligo.skymap/-/merge_requests/288Draft: Switch from legacy icc compiler to icx2023-06-27T09:52:21ZLeo P. SingerDraft: Switch from legacy icc compiler to icxicx is Intel's new optimizing compiler based on Clang.icx is Intel's new optimizing compiler based on Clang.https://git.ligo.org/lscsoft/ligo.skymap/-/merge_requests/271Draft: Replace bayestar_realize_coincs.py2022-03-16T17:24:57ZPippa ColeDraft: Replace bayestar_realize_coincs.pyReplace bayestar_realize_coincs.py script to include effects of earth rotation. We have made adaptations to lines 127 - 159. The main change is that we have included a time shift in the antenna function, so that Fplus and Fcross are now ...Replace bayestar_realize_coincs.py script to include effects of earth rotation. We have made adaptations to lines 127 - 159. The main change is that we have included a time shift in the antenna function, so that Fplus and Fcross are now arrays corresponding to time samples given by 'tlist' (line 149).
One thing we are particularly unsure of is how to feed this into the snr series - currently we do this by manipulating the time samples to match those of snr_series (line 150).https://git.ligo.org/lscsoft/ligo.skymap/-/merge_requests/264Clarify a few details of fields in the FITS header2023-06-29T19:21:29ZTito Dal CantonClarify a few details of fields in the FITS headerhttps://git.ligo.org/lscsoft/ligo.skymap/-/merge_requests/241WIP: Add network SNR threshold to BAYESTAR likelihood2021-09-29T13:26:59ZLeo P. SingerWIP: Add network SNR threshold to BAYESTAR likelihoodBAYESTAR's likelihood now includes the network SNR threshold for
detection. The default value is zero. For nonzero values, the likelihood
broadens as the network SNR approaches its threshold value.
The network SNR threshold is set using...BAYESTAR's likelihood now includes the network SNR threshold for
detection. The default value is zero. For nonzero values, the likelihood
broadens as the network SNR approaches its threshold value.
The network SNR threshold is set using the optional
``--net-snr-threshold`` command line argument, which is understood
by the ``bayestar-localize-coincs`` and
``bayestar-localize-lvalert`` scripts.
Note, output of Bayes factors for signal-versus-noise (BNS) and
coherence-versus-incoherence (BCI) are not working yet with this
patch.https://git.ligo.org/lscsoft/ligo.skymap/-/merge_requests/237Update default cosmological model to Planck182024-01-24T01:00:02ZLeo P. SingerUpdate default cosmological model to Planck18This also bumps the minimum version of Astropy to 4.2.1.
Fixes #19.This also bumps the minimum version of Astropy to 4.2.1.
Fixes #19.https://git.ligo.org/lscsoft/ligo.skymap/-/merge_requests/214Move optional dependencies to extras2020-08-20T22:42:12ZLeo P. SingerMove optional dependencies to extrashttps://git.ligo.org/lscsoft/ligo.skymap/-/merge_requests/210Use GitLab CI caching instead of making intermediate Docker containers2020-08-17T23:12:10ZLeo P. SingerUse GitLab CI caching instead of making intermediate Docker containershttps://git.ligo.org/lscsoft/ligo.skymap/-/merge_requests/207Adjust tqdm smoothing for more accurate estimates with multiprocessing on2020-08-04T14:24:10ZLeo P. SingerAdjust tqdm smoothing for more accurate estimates with multiprocessing onhttps://git.ligo.org/lscsoft/ligo.skymap/-/merge_requests/195WIP: Port to PBS and other job schedulers using dask2020-07-25T00:10:22ZLeo P. SingerWIP: Port to PBS and other job schedulers using daskFor all tools that use Python multiprocessing for parallelism, add the `--cluster` option to support using a job scheduler such as HTCondor or PBS to start remote workers.
- Currently, while this is a work in progress, only `bayestar-in...For all tools that use Python multiprocessing for parallelism, add the `--cluster` option to support using a job scheduler such as HTCondor or PBS to start remote workers.
- Currently, while this is a work in progress, only `bayestar-inject` supports this.
- Documentation on cluster configuration needed.
## Proposed command line arguments
A command line tool called `foobar` would support these options:
```sh
# Local, single process
foobar
foobar -j 1
# Local, as many processes as there are CPUs
foobar -j
# Local, specific number of processes
foobar -j 16
# Remote
foobar -j 16 --cluster htcondor
foobar -j 16 --cluster pbs
```https://git.ligo.org/lscsoft/ligo.skymap/-/merge_requests/186Run CI tests in parallel using pytest-xdist2022-05-27T18:51:15ZLeo P. SingerRun CI tests in parallel using pytest-xdisthttps://git.ligo.org/lscsoft/ligo.skymap/-/merge_requests/185Remove integrator region1 to cut down on branching2020-05-24T16:03:56ZLeo P. SingerRemove integrator region1 to cut down on branchinghttps://git.ligo.org/lscsoft/ligo.skymap/-/merge_requests/106Change default sky map file extension to .multiorder.fits2019-05-17T20:51:42ZLeo P. SingerChange default sky map file extension to .multiorder.fitshttps://git.ligo.org/lscsoft/ligo.skymap/-/merge_requests/77WIP: Put all work in one big parallel region2018-11-16T19:57:39ZLeo P. SingerWIP: Put all work in one big parallel regionThis is supposed to be faster because it avoids creating and
destroying threads repeatedly. But I don't think that it actually
is faster because OpenMP capable compilers are smarter than they
used to be.This is supposed to be faster because it avoids creating and
destroying threads repeatedly. But I don't think that it actually
is faster because OpenMP capable compilers are smarter than they
used to be.Leo P. SingerLeo P. Singerhttps://git.ligo.org/lscsoft/ligo.skymap/-/merge_requests/67Add tool for planning galaxy-targeted search2018-11-16T21:18:20ZLeo P. SingerAdd tool for planning galaxy-targeted searchLeo P. SingerLeo P. Singer