Some Improvement in aspect processing

i4Ds / STIX-GSW

STIX Ground-Analysis Software Package

11 stars 15 forks source link

Some Improvement in aspect processing #179

Closed FredSchuller closed 8 months ago

FredSchuller commented 9 months ago

Updates in several procedures called to process SAS data, including:

added a procedure stx_remove_bad_sas_data
updates in SAStest to call procedures with name stx...
store error codes in data.ERROR (partly addresses issue #129). So far only these codes are implemented:
- SUN_TOO_FAR (when distance > 0.75 AU)
- NO_ASPECT_SOL (if for some reason the computation resulted in NaN)
- SAS_DATA_WRONG_DURATION (for HK data where duration deviates from the expected 64s, e.g. for duplicated entries)

FredSchuller commented 9 months ago

More changes to implement more error codes:

SUN_TOO_BIG (if solar image radius larger than the range covered in the simulated data)
SAS_SOL_TOO_FAR (if solution found at more that 1.1 R_sol, i.e. far off the solar limb)

In addition, field sas_ok is now implemented and set to 1 only when no error was found.

samaloney commented 9 months ago

Since we will fix the HK data maybe the SAS_DATA_WRONG_DURATION isn't needed? Also all the status message seem different if I'd suggest either in clue SAS_ in all or remove from all

Current	Sun
SUN_TOO_FAR	SUN_TOO_SMALL / SUN_TOO_FAR
SUN_TOO_BIG	SUN_TOO_BIG / SUN_TOO_CLOSE
SAS_SOL_TOO_FAR	OFFPOINT_TOO_LARGE
NO_ASPECT_SOL	NO_SOLUTION

Also not sure how the plan to encode but a number would be better 0, 1, 2, 3 can be stores as two bits

FredSchuller commented 9 months ago

I'm happy to rename the error messages as you suggest. Still, I would keep the possibility for WRONG_DURATION (we can use a shorter text if you prefer) as long as the issue is not fixed - and it may happen again in the future...

The plan is to include this error message AND a boolean (or short integer) "SAS_OK" in the AUX fits file. The imaging pipeline will only make use of the later, to filter out every SAS measurement that is not OK, for any reason.

FredSchuller commented 8 months ago

@samaloney I have updated the error messages as per your suggestion. From my point of view, this PR could be merged now.

samaloney commented 8 months ago

Yea looks good to me but I'd like to get @nicHoch input. In particular I'm not sure including both the string and error code/boolean make sense not in the fits file itself anyway. Also needs a rebase to resolved the conflicts with current master.

nicHoch commented 8 months ago

it was planed as String in order to provide the user with a verbose error reason. most of the time the string will be empty. but we could also change it to ENUM and add the verbose messages to the column description