LCOGT / mop

Microlensing Observation Portal
GNU General Public License v3.0
0 stars 7 forks source link

PriorityEvents list missing OGLE and other events #141

Closed rachel3834 closed 4 months ago

rachel3834 commented 5 months ago

The priority events list was recently refactored to minimize DB query load. Post the refactor, only Gaia targets show up in the priority target list.

Guess is that this is related to the change in how we are handling the target selection for this page. The code is now doing more sophisticated filtering on DB querysets rather than the post-hoc interpretation of the TargetExtras. As a result, the extras for is_YSO, is_QSO and is_galaxy are being properly handled as the string parameters they are declared to be, and are not subject to as much hand-holding.

These parameters should ideally be boolean, and we should ensure that they are set properly for all targets in the DB.

KKruszynska commented 5 months ago

I will check in the morning if the is_YSO, is_QSO, etc. fields are initialized for OGLE targets. I introduced them at the end of OGLE's observing season, so this might be the cause.

KKruszynska commented 5 months ago

I just checked. In the csv file that can be obtained through Targets -> Export Filtered Targets that I downloaded this morning for OGLE bulge events from 2023, only the last ~4 targets of 2023 have is_YSO, is_QSO, is_galaxy filled with any value. See: Screenshot from 2024-02-22 18-14-34

Ticked or unticked box in this screen shot indicates a True or False value, while nothing indicated an empty field (None).

rachel3834 commented 5 months ago

Many thanks Katarzyna! I will create a quick management command to standardise the database entries.

On Thu, Feb 22, 2024 at 6:15 PM KKruszynska @.***> wrote:

I just checked. In the csv file that can be obtained through Targets -> Export Filtered Targets that I downloaded this morning for OGLE bulge events from 2023, only the last ~4 targets of 2023 have is_YSO, is_QSO, is_galaxy filled with any value. See: Screenshot.from.2024-02-22.18-14-34.png (view on web) https://github.com/LCOGT/mop/assets/36246703/b3b1808c-22ed-403d-883d-087644fceedc

Ticked or unticked box in this screen shot indicates a True or False value, while nothing indicated an empty field (None).

— Reply to this email directly, view it on GitHub https://github.com/LCOGT/mop/issues/141#issuecomment-1960643669, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACPJA3F3FCYHRU6SRKLYRODYU73UTAVCNFSM6AAAAABDV4CXJOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRQGY2DGNRWHE . You are receiving this because you were assigned.Message ID: @.***>

rachel3834 commented 5 months ago

Reviewing the codebase, the reason for this issue is that we only check to see if a target is a known YSO, QSO or galaxy if it detected from Gaia. This is an understandable approach because OGLE and MOA provide very carefully vetted alert streams.
The net result is that the check isn't made for OGLE and MOA targets, so Targets from those surveys may not have extra_field parameters for "is_YSO" etc and if they do, the entry may be set to the default, which is declared in settings.py to be an empty string rather than 'false'. (Note that the parameter is defined to be a string rather than a boolean, but the gaia_classifier sometimes handles it as a boolean).

Although OGLE and MOA targets don't strictly need this type of vetting, following the Principle of Least Astonishment, it makes sense for all Targets in MOP to have an evaluated entry for these parameters. This allows us to use the keywords to search the database most efficiently, and eliminates a lot of time-consuming double checks.

I think it also makes sense for these parameters to be boolean for consistency, since we don't need to store e.g. a YSO identifier - we just need to know if the object has one or not. To make the migration of the database cleaner, I will define new boolean extra_field parameters for these entries, then run a management command to check all Targets and set them accordingly. Then the old extra_field entries can be cleanly deleted but minimize chances of mistakes on my part.

Tasks:

KKruszynska commented 5 months ago

Yes, these parameters should be boolean. I can fix gaia_classifier to be consistent afterwards.

rachel3834 commented 4 months ago

I've refactored the gaia_classifier and ported the code to check for QSO, YSO, TNS etc from this code to the data harvesters. This means that these checks are now performed for all targets, not just Gaia ones. In the process, I updated the gaia_classifier to use the optimized querytools with MicrolensingEvent objects. This includes the logic to set the classification and category flags appropriately upon ingest, meaning that subsequent queries for 'all alive microlensing targets' more reliably receive exactly that.

rachel3834 commented 4 months ago

The migration of the key-value pairs is complete for both mop-dev and mop-prod, and the codebase has been updated to use the new keywords. This issue is fixed in version 4.3.19.