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.

Sign up for GitHub

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

Jump to bottom

Use proseco for P_ACQ #272

Merged
merged 23 commits into from
Jan 17, 2019
Merged

Use proseco for P_ACQ #272

merged 23 commits into from
Jan 17, 2019

Conversation

jeanconn
Copy link
Contributor

@jeanconn jeanconn commented Jan 5, 2019 •
edited
Loading

Use proseco for P_ACQ

Closes #268
Closes #283

@jeanconn jeanconn changed the title Use proseco for P_ACQ WIP: Use proseco for P_ACQ Jan 5, 2019
@jeanconn jeanconn changed the title WIP: Use proseco for P_ACQ Use proseco for P_ACQ Jan 10, 2019
@jeanconn jeanconn mentioned this pull request Jan 10, 2019
Closed
@jeanconn jeanconn requested a review from taldcroft January 10, 2019 20:09
taldcroft
taldcroft reviewed Jan 13, 2019
View reviewed changes
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.

I just started looking at this. Since this is new code I'd like to see a lot more code comments and function docstrings before I review.

starcheck/calc_ccd_temps.py Outdated Show resolved Hide resolved
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

Regarding "I just started looking at this. Since this is new code I'd like to see a lot more code comments and function docstrings before I review." Thanks that's fine. I just didn't want to get too far down any path only to find out you hated a concept / strategy.

taldcroft
taldcroft reviewed Jan 14, 2019
View reviewed changes
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.

OK I've at least looked through all this somewhat carefully.

starcheck/src/lib/Ska/Starcheck/Obsid.pm Show resolved Hide resolved
starcheck/src/lib/Ska/Starcheck/Obsid.pm Outdated Show resolved Hide resolved
starcheck/src/lib/Ska/Starcheck/Obsid.pm Show resolved Hide resolved
starcheck/src/lib/Ska/Starcheck/Obsid.pm Outdated Show resolved Hide resolved
starcheck/src/lib/Ska/Starcheck/Obsid.pm Outdated Show resolved Hide resolved
starcheck/src/lib/Ska/Starcheck/Obsid.pm Outdated Show resolved Hide resolved
starcheck/src/lib/Ska/Starcheck/Obsid.pm Outdated Show resolved Hide resolved
starcheck/src/lib/Ska/Starcheck/Obsid.pm Outdated Show resolved Hide resolved
This was referenced Jan 15, 2019
Merged
Closed
jeanconn added 12 commits January 16, 2019 20:53
@jeanconn
Add ccd_temp_acq to values calculated in calc_ccd_temps
27a6d50
@jeanconn
Use proseco for P_ACQ calculations
46c8943
@jeanconn
Remove FigureOfMerit from Makefile
003cdc6
@jeanconn
Use maneuver summary maneuver angle instead of calculating it
4be5b72
@jeanconn
Handle missing data (ReOpen schedule beginning) for proseco probs
911d4a3
@jeanconn
Use the 'last' maneuver for maneuver angle (for segmented manvrs for …
a1e2276 
…now)
@jeanconn
Skip trying for proseco args for NONE obsids
0e75f3f
@jeanconn
Recharacterize big box warning as yellow_warn
887e7bf
@jeanconn
Add comment for empty hover-over acq prob and simplify a bit
98dc5bd
@jeanconn
Remove big box check and set proseco probs earlier in loop in starche…
2082c6d 
…ck.pl
@jeanconn
Assume the best
998ff46 
This change sets proseco_probs to assume that the supplied acq ids all
have appropriate acq probs.  It also assumes that a manuever angle
is set for each observations.
@jeanconn
Update comments/docstrings (and fiddle with whitespace)
a01658b
@jeanconn
Copy link
Contributor Author

I think this is good to go. The last commit, 945fa19, is the change since the meeting today.

@jeanconn
Copy link
Contributor Author

Is there anything else I should be checking wrt to P_ACQ and P2? Will review vs the pkls that we have.

@taldcroft taldcroft added this to the 13.0 proseco milestone Jan 17, 2019
@taldcroft
Copy link
Member

I checked 4e289bf and the rest by running make test with a temporarily edited version with P2_red=5 or 6. This generated the expected yellow and red warnings.

BTW make test seems to require that ROOT_FLIGHT is defined outside the test script.

@taldcroft taldcroft mentioned this pull request Jan 17, 2019
Merged
@jeanconn jeanconn merged commit ea3eb25 into master Jan 17, 2019
@jeanconn jeanconn deleted the proseco_probs branch January 17, 2019 13:38
@javierggt javierggt mentioned this pull request Apr 12, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@taldcroft taldcroft taldcroft approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
13.0 proseco
Development

Successfully merging this pull request may close these issues.
2 participants
@jeanconn @taldcroft