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

Add fid offset support #431

Merged
merged 14 commits into from
Dec 5, 2023
Merged

Add fid offset support #431

merged 14 commits into from
Dec 5, 2023

Conversation

jeanconn
Copy link
Contributor

@jeanconn jeanconn commented Oct 18, 2023

Description

Moves "expected" position of fid lights to be their positions with chandra_aca.drift.get_fid_offset applied.

The expected positions are used to check if there is a commanded ON fid light within fid-light-search-hw of th expected position and to check if there is a fid light in an acquisition box

This also removes < -14C warning (which was a proxy for fid position).

Interface impacts

Testing

Unit tests

  • Mac

Independent check of unit tests by [REVIEWER NAME]

  • [PLATFORM]:

Functional tests

Output in https://icxc.cfa.harvard.edu/aspect/test_review_outputs/starcheck/starcheck-pr431/

sandbox_starcheck run out of ska3-masters

/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2022/NOV2122/oflsa/ -out nov2122a_25hw # with 25hw box artificially set on Obsid.pm
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2022/NOV2122/oflsa/ -out nov2122a_40hw # with 40hw box (PR value)
/proj/sot/ska3/flight/bin/skare starcheck -dir /data/mpcrit1/mplogs/2022/NOV2122/oflsa/ -out nov2122a_flight
diff nov2122a_40hw.txt nov2122a_flight.txt > nov2122a_diff.txt
/proj/sot/ska/bin/diff2html nov2122a_flight.txt nov2122a_40hw.txt > nov2122a_diff.html

@jeanconn jeanconn changed the title WIP: Add fid offset support Add fid offset support Dec 4, 2023
@jeanconn jeanconn marked this pull request as ready for review December 4, 2023 13:16
@jeanconn jeanconn requested a review from taldcroft December 4, 2023 13:16
Copy link
Member

@taldcroft taldcroft left a comment

Choose a reason for hiding this comment

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

This looks good with just a couple of minor comments and removing the sanity check. We could ask Matt but I'm reasonably confident the CCD model settling temperature is well above -20C everywhere and somewhat below +5C everywhere for at least the next year. So this check basically cannot be triggered.

starcheck/src/lib/Ska/Starcheck/Obsid.pm Outdated Show resolved Hide resolved
starcheck/src/lib/Ska/Starcheck/Obsid.pm Outdated Show resolved Hide resolved
@jeanconn
Copy link
Contributor Author

jeanconn commented Dec 5, 2023

Functional tests rerun at c354581 .

Copy link
Member

@taldcroft taldcroft left a comment

Choose a reason for hiding this comment

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

Looks good now!

@jeanconn jeanconn merged commit 6895430 into master Dec 5, 2023
@jeanconn jeanconn deleted the fid-drift branch December 5, 2023 22:30
@javierggt javierggt mentioned this pull request Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants