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

Experiment with GPO on 2023-02-07 #33

Merged
merged 21 commits into from
Mar 20, 2023
Merged

Experiment with GPO on 2023-02-07 #33

merged 21 commits into from
Mar 20, 2023

Conversation

mrakitin
Copy link
Member

Works in tandem with NSLS-II/blop#5.

mrakitin
mrakitin commented Feb 17, 2023
View reviewed changes
Copy link
Member Author

@mrakitin mrakitin left a comment

Choose a reason for hiding this comment

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

Here is my preliminary review. We won't merge the PR until after we are done with our experiment on 2/18-19.

startup/00-startup.py Outdated Show resolved Hide resolved
startup/22-shutters.py Outdated Show resolved Hide resolved
startup/22-shutters.py Show resolved Hide resolved
startup/92-optimization.py Outdated Show resolved Hide resolved
startup/98-suspenders.py Outdated Show resolved Hide resolved
@mrakitin
Copy link
Member Author

I'll resolve the merge conflict.

@mrakitin
Copy link
Member Author

I resolved the conflicts, and the PR should be mergeable now.

@mrakitin
Copy link
Member Author

We decided to keep this PR in a Draft state until after we are done with our experiment on 2/18-19. We'll stack the relevant code changes here.

@mrakitin mrakitin closed this Feb 18, 2023
@mrakitin mrakitin reopened this Feb 18, 2023
@mrakitin mrakitin marked this pull request as ready for review February 19, 2023 19:21
@mrakitin mrakitin mentioned this pull request Feb 19, 2023
Merged
thomaswmorris and others added 16 commits February 21, 2023 16:21
@thomaswmorris @mrakitin
The code for optimization using GPyTorch + add missing beamline ophyd…
7386b8a 
… objects
@thomaswmorris @mrakitin
Updates from the experiment on 2/8/23
3a2992a
@mrakitin
CI: use 2023-1.1 configs for testing
e8822aa
@mrakitin
CI: use the PR no.5 branch of bloptools
e8af7e9
@mrakitin
CI: fix the src dir location
fdc1c1a
@mrakitin
CI: fix the drop-in workflow
dd07db3
@mrakitin
CI: do not fail on unbound variables
e5cd995
@mrakitin
CI: use the 2023-1.2 conda envs (with ortools)
68e9d2d
@mrakitin
Account for the new locations of the de modules
f09fc19
@mrakitin
Coding on 2/18/23
bab5698
@mrakitin
CI: update the bloptools installation procedure on top of the conda…
55a15c1 
… env
@mrakitin
CI: use the 2023-1.2 configs from the main branch of templates
6ca5a90
@mrakitin
Address PR review suggestions/add comments about the shutter class
aab59a2
@thomaswmorris @mrakitin
Update 98-suspenders.py
5eb2432
@mrakitin
Better processing of suspended installation
1ba27ac
@mrakitin
Use an `auto_alignment_mode() function to switch the modes and do rel…
0d9b36f 
…evant imports
@mrakitin mrakitin requested a review from AbbyGi February 22, 2023 02:17
jklynch
jklynch requested changes Feb 22, 2023
View reviewed changes
Copy link
Contributor

@jklynch jklynch left a comment

Choose a reason for hiding this comment

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

Only a few minor requests.

startup/92-optimization.py Show resolved Hide resolved
startup/92-optimization.py Outdated Show resolved Hide resolved
startup/92-optimization.py Outdated Show resolved Hide resolved
@mrakitin mrakitin requested a review from jklynch February 22, 2023 18:39
@mrakitin
Copy link
Member Author

@jklynch, are you OK with the changes now?

thomaswmorris
thomaswmorris approved these changes Mar 17, 2023
View reviewed changes
Copy link
Contributor

@thomaswmorris thomaswmorris 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, I don't see any reason why the GPO won't work (but I can't speak to whether or not it affects other things)

jklynch
jklynch approved these changes Mar 17, 2023
View reviewed changes
Copy link
Contributor

@jklynch jklynch 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 to me!

@mrakitin
Copy link
Member Author

Thanks for your reviews! Please feel free to merge.

@jklynch jklynch merged commit 28e9d34 into master Mar 20, 2023
@jklynch jklynch deleted the expt-gpo-20230207 branch March 20, 2023 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomaswmorris thomaswmorris thomaswmorris approved these changes

@jklynch jklynch jklynch approved these changes

@AbbyGi AbbyGi Awaiting requested review from AbbyGi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mrakitin @jklynch @thomaswmorris