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 get_bpms #100

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

add get_bpms #100

wants to merge 4 commits into from

Conversation

awegsche
Copy link
Contributor

@awegsche awegsche commented Sep 6, 2023

Adds a description on how to get a list of consistant bad BPMs

@tpersson please check if this procedure is complete and if there's anything important to add (like the fact that we should sample a certain amount of different, representative measurements)

I copied the whole script in here but I am sure there's a better place to put this. I am open for suggestions

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.

Nice addition, I'll have a look at the omc3 PR shortly.

I'm not a fan of including the script here. I would rather make it flexible in omc3 if it isn't already and avoid the duplication if possible. Waiting for other opinions on this.

@@ -0,0 +1,148 @@
# Get a list of Bad BPMs

Every now and then BI asks for a list of BPMs that cause trouble for our measurements so that they can take a closer look during the next [(E?YET|L)S](https://regex101.com/r/ShN5hC/3).
Copy link
Member

Choose a reason for hiding this comment

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

I don't feel like a hyperlink to regex101 is warranted, I'd rather see a simple admonition pop up explaining the meaning of YETS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ooooh, I liked this little joke

Output will be written to `"bad_bpms.txt"`

## Usage:
- [ ] Define a set of representative measurements (those shoulf probably span different optics configurations and different analysis purposes)
Copy link
Member

Choose a reason for hiding this comment

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

little typo here


## Usage:
- [ ] Define a set of representative measurements (those shoulf probably span different optics configurations and different analysis purposes)
- [ ] Make sure that the measurements have the desired cleaning method applied.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe let's switch to "rerun the analysis" rather than the measurement

- [ ] Make sure that the measurements have the desired cleaning method applied.
If needed rerun the measurements with the GUI.
- [ ] Adapt the `DATES` list at the beginning of this script accordingly
- [ ] a) Run this script in `OP_DATA` or
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather say to run the script in the directory where GUI outputs are located, and specify that it can be OP_DATA if looking on the TN at the usual place.

@awegsche
Copy link
Contributor Author

Yeah, I don't know where to put the script.

I don't want it to be a fully general script with argparse etc, that would be too rigid. I prefer it being a template where everyone can just change the paths and logic inside, run and get the needed output.

@fsoubelet
Copy link
Member

Hmm then in this case maybe it shouldn't be included in omc3 at all, and we only provide a template for the user on the website.

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