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

BPM Calibration #51

Merged
merged 54 commits into from Apr 22, 2021
Merged

BPM Calibration #51

merged 54 commits into from Apr 22, 2021

Conversation

Mael-Le-Garrec
Copy link
Contributor

Script to compute the calibration factors using several methods:

  • β from phase and its fit
  • Dispersion and its fit

This code is based on get_ballistic_calibration.py and get_dispersion_calibration.py

Fixes #27

@Mael-Le-Garrec Mael-Le-Garrec marked this pull request as ready for review November 26, 2020 15:48
JoschD
JoschD reviewed Nov 26, 2020
View reviewed changes
pylhc/BPM_calibration.py Outdated Show resolved Hide resolved
JoschD
JoschD requested changes Nov 26, 2020
View reviewed changes
pylhc/BPM_calibration.py Outdated Show resolved Hide resolved
pylhc/constants/calibration.py Show resolved Hide resolved
@fsoubelet
Copy link
Member

fsoubelet commented Apr 21, 2021 •
edited

Any news @Mael-Le-Garrec? This seems ready to go unless you have more modifications in mind.

@Mael-Le-Garrec
Copy link
Contributor Author

Any news @Mael-Le-Garrec? This seems ready to go unless you have more modifications in mind.

Everything's ok for me

fsoubelet
fsoubelet previously approved these changes Apr 22, 2021
View reviewed changes
Copy link
Member

@fsoubelet fsoubelet left a comment

Choose a reason for hiding this comment

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

All good for me with this small addition. Will ping @JoschD and with his green light, we're good.

@fsoubelet fsoubelet requested a review from JoschD April 22, 2021 15:38
JoschD
JoschD reviewed Apr 22, 2021
View reviewed changes
pylhc/calibration/beta.py Outdated Show resolved Hide resolved
Mael-Le-Garrec
Mael-Le-Garrec commented Apr 22, 2021
View reviewed changes
Copy link
Contributor Author

@Mael-Le-Garrec Mael-Le-Garrec left a comment

Choose a reason for hiding this comment

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

Just removed

pylhc/calibration/beta.py Outdated Show resolved Hide resolved
@JoschD
Copy link
Member

JoschD commented Apr 22, 2021

Another problem will be the test BSRT that michael fixed, but only in my branch...

fsoubelet
fsoubelet approved these changes Apr 22, 2021
View reviewed changes
Copy link
Member

@fsoubelet fsoubelet left a comment

Choose a reason for hiding this comment

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

Thanks @JoschD for the catch and the new baselines! Let's merge.

@fsoubelet fsoubelet dismissed lmalina’s stale review April 22, 2021 17:59

Old review, changes were included.

@fsoubelet fsoubelet merged commit 7d9f037 into master Apr 22, 2021
@fsoubelet fsoubelet deleted the enhancement/27/bpm_calibration branch April 22, 2021 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lmalina lmalina lmalina left review comments

@JoschD JoschD JoschD left review comments

@mihofer mihofer mihofer left review comments

@fsoubelet fsoubelet fsoubelet approved these changes

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

Successfully merging this pull request may close these issues.

Add scripts to calculate BPM calibrations
5 participants
@Mael-Le-Garrec @mihofer @fsoubelet @JoschD @lmalina