search

skyportal / gwemopt

Gravitational-wave Electromagnetic Optimization
GNU General Public License v3.0
19 stars 34 forks source link

Deal with no compatible galaxies, add AGN_flag for mangrove #59

Closed jgducoin closed 4 years ago

jgducoin commented 4 years ago

For the event S200106av, looking at the observation plan produced (cf attached image)Screenshot from 2020-01-14 10-05-02, it looks crazy. The problem comes from the fact that the skymap and distance error are so small that there are no compatible galaxies. Looking at the gwemopt code it seems that there is a "security" (here: https://github.com/mcoughlin/gwemopt/blob/master/gwemopt/catalog.py#L251) which put Sloc to 1 for all galaxies in such case. This led to point everywhere in the sky. This is probably a marginal effect because such skymap comes only from a non-astrophysical event, but it looks weird for people not aware of that to receive such obsplan.

Changes proposed in this pull request:

Other change:

mcoughlin commented 4 years ago

So I think it still makes sense to set the numbers to 1 but also print some kind of big warning when this occurs? Folks will still run the code, and other codes will fail if nothing comes out of the catalogs call. Maybe better is to have ToO_Manager interpret this case?

dcorre commented 4 years ago

Hi @mcoughlin , could we simply add a new keyword CompatibleGal (True/False) in the catalog.py's output dictionnaries map_struct or catalog_struct?

mcoughlin commented 4 years ago

I think this could work. Are you proposing to leave the non-zero catalog numbers still to keep backwards compatibility?

dcorre commented 4 years ago

Yes, if returning an empy dictionnary may crash the code some other people are using, we could simply add a flag in the output dict, and use its value in ToO_manager. If this fine for you, @jgducoin can implement it.

jgducoin commented 4 years ago

Yes I will put back the "security" (here: https://github.com/mcoughlin/gwemopt/blob/master/gwemopt/catalog.py#L251) and add the keyword.

jgducoin commented 4 years ago

@mcoughlin done in the last commit. Should work as expected now.