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

Function get_hpc_info should return interpolated values when time range fully between two measurements #151

Open
FredSchuller opened this issue Dec 5, 2024 · 5 comments · May be fixed by #152
Assignees
Labels
bug Something isn't working

Comments

@FredSchuller
Copy link

As first noticed by @jajmitchell the function get_hpc_info sometimes returns wrong results that show strong deviation compared to results obtained one minute earlier or one minute later.
The reason is: when that function doesn't find any valid pointing measurement within the input time range, it falls back to the spacecraft pointing. But it should perform an interpolation between the previous and the next measurements (if available and marked as good, i.e. sas_ok=1) for better results.

@FredSchuller
Copy link
Author

Here is a small script to illustrate that issue:

from astropy.time import Time
from stixpy.coordinates.transforms import get_hpc_info

t1 = Time("2022-03-28T18:43:00")
t2 = Time("2022-03-28T18:44:00")
t3 = Time("2022-03-28T18:45:00")
t4 = Time("2022-03-28T18:46:00")

roll, solo_xyz, ptg_1 = get_hpc_info(t1, t2)
roll, solo_xyz, ptg_2 = get_hpc_info(t2, t3)
roll, solo_xyz, ptg_3 = get_hpc_info(t3, t4)
print(ptg_1, ptg_2, ptg_3)

All three results should be very similar...

@samaloney samaloney self-assigned this Dec 5, 2024
@samaloney samaloney added the bug Something isn't working label Dec 5, 2024
@samaloney
Copy link
Member

samaloney commented Dec 5, 2024

So the code does do the interpolation but ignores it because there is no SAS solution in the time range and the code mistakenly interprets this as a bad SAS solution. I can fix this relatively easily but it does raise another question what you end up interpolating between two bad SAS solutions?

In the second case in the example L123-124 indices is empty so good_sas is empty which cases the spacecraft solution to be used via L137

sas_x = np.interp(x, xp, aux["y_srf"])
sas_y = np.interp(x, xp, aux["z_srf"])
good_solution = np.where(aux[indices]["sas_ok"] == 1)
good_sas = aux[good_solution]
# Convert the spacecraft pointing to STIX frame
rotated_yaw = -yaw * np.cos(roll) + pitch * np.sin(roll)
rotated_pitch = yaw * np.sin(roll) + pitch * np.cos(roll)
spacecraft_pointing = np.vstack([STIX_X_SHIFT + rotated_yaw, STIX_Y_SHIFT + rotated_pitch]).T
stix_pointing = spacecraft_pointing
sas_pointing = np.vstack([sas_x + STIX_X_OFFSET, -1 * sas_y + STIX_Y_OFFSET]).T
pointing_diff = np.sqrt(np.sum((spacecraft_pointing - sas_pointing) ** 2, axis=0))
if np.all(np.isfinite(sas_pointing)) and len(good_sas) > 0:
if np.max(pointing_diff) < 200 * u.arcsec:
logger.info(f"Using SAS pointing: {sas_pointing}")
stix_pointing = np.where(pointing_diff < 200 * u.arcsec, sas_pointing, spacecraft_pointing)
else:
warnings.warn(
f"Using spacecraft pointing: {spacecraft_pointing}" f" large difference between SAS and spacecraft."
)
else:
warnings.warn(f"SAS solution not available using spacecraft pointing: {stix_pointing}.")

@FredSchuller
Copy link
Author

The interpolated value should be used only if the aspect solutions before and after are both "ok".
In the IDL version (see stx_create_auxiliary_data), we first compute the time of the nearest neighbours t_before and t_after and then evaluate sas_ok as the logical operation: sas_ok[before] AND sas_ok[after].

@samaloney
Copy link
Member

I might just filter out all the bad SAS solutions earlier, is the a case where they might be needed?

@FredSchuller
Copy link
Author

No, I think it's a safe approach.

@samaloney samaloney linked a pull request Dec 9, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants