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

1020 bug get spacecraft spin phase always returns nans #1021

Conversation

subagonsouth
Copy link
Contributor

Change Summary

Overview

When implementing a new function for getting instrument spin phase, I found there was a bug in get_spacecraft_spin_phase() causing it to always return NaNs. np.searchsorted returns indices such that a[i-1] <= v < a[i]. This means that the correct row in the dataframe to use to compute the spin phase is i-1.

I was having a hard time updating the tests after the fix due to lack of control of the dataframe produced by the generate_spin_data() fixture so I made my own fake spin data CSV which allows testing of all branches in the function.

Closes: #1020

Add csv file for testing all branches of get_spacecraft_spin_phase logic
Update test coverage for get_spacecraft_spin_phase logic
@subagonsouth subagonsouth added the SPICE Related to SPICE label Oct 15, 2024
@subagonsouth subagonsouth added this to the Oct 2024 milestone Oct 15, 2024
@subagonsouth subagonsouth self-assigned this Oct 15, 2024
Copy link
Contributor

@tech3371 tech3371 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me! Thank you for fixing this!

@@ -207,7 +207,7 @@ def get_spacecraft_spin_phase(
)
input_start_time = query_met_times.min()
input_end_time = query_met_times.max()
if input_start_time < spin_df_start_time or input_end_time > spin_df_end_time:
if input_start_time < spin_df_start_time or input_end_time >= spin_df_end_time:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for fixing those! I tried to be careful but still missed those two.

# 1 good spin
9,135,0,15.0,1,1,0,0
# Thruster firing on
10,150,0,15.0,1,1,0,1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach was much simpler than what I had and you were able to test for different scenario. Nice!

(np.array([110, 111]), np.full(2, np.nan)),
# Test for time in missing spin
(65, np.nan),
(np.array([65.1, 66]), np.full(2, np.nan)),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

much better tests!

@subagonsouth subagonsouth merged commit 019d7f3 into IMAP-Science-Operations-Center:dev Oct 16, 2024
17 checks passed
@subagonsouth subagonsouth deleted the 1020-bug-get_spacecraft_spin_phase-always-returns-nans branch October 16, 2024 20:21
@tech3371 tech3371 removed this from the Oct 2024 milestone Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SPICE Related to SPICE
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

BUG - get_spacecraft_spin_phase always returns nans
2 participants