Skip to content

Feature/mfx is lcls2 #281

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
Merged
silkenelson merged 13 commits into from
May 6, 2025
Merged

Feature/mfx is lcls2 #281

silkenelson merged 13 commits into from
May 6, 2025

Conversation

@silkenelson
Copy link
Collaborator

@silkenelson silkenelson commented May 6, 2025

changes for RIXS pedestals in takepeds
determine LCLS1 vs LCLS2 experiment
use of the former script in makepeds
makepeds_psana2 also for axis (to be worked on more)
small bug fixes
use of segment number currently needed in deploy step

silkenelson added 4 commits May 5, 2025 23:26
@silkenelson
print message&makepeds command when takepeds is done, but before the …
1da39d0 
…elog post; add timedrun for pedestals runs in RIXS
@silkenelson
if passing experiment name, use exp-name to determine hutch if needed…
78e2d91 
…. Add option to see if this is an LCLS1 or LCLS2 experiment
@silkenelson
use experiment data file names to determine the DAQ used (workf ro MF…
c5f8905 
…X that has runs using either DAQ)
@silkenelson
fix a couple small bugs, add axis makepeds (still in progress), add c…
4b60688 
…heck to interactive jungfrau running, deal with multi-segment detectors (may change soon)
@silkenelson silkenelson requested review from a team as code owners May 6, 2025 06:34
KaushikMalapati
KaushikMalapati reviewed May 6, 2025
View reviewed changes
@silkenelson @KaushikMalapati
Update scripts/makepeds_psana2
1198a80 
Co-authored-by: KaushikMalapati <80156796+KaushikMalapati@users.noreply.github.com>
KaushikMalapati
KaushikMalapati reviewed May 6, 2025
View reviewed changes
@silkenelson @KaushikMalapati
Update scripts/makepeds
28b2737 
Co-authored-by: KaushikMalapati <80156796+KaushikMalapati@users.noreply.github.com>
KaushikMalapati
KaushikMalapati reviewed May 6, 2025
View reviewed changes
KaushikMalapati
KaushikMalapati reviewed May 6, 2025
View reviewed changes
KaushikMalapati
KaushikMalapati reviewed May 6, 2025
View reviewed changes
KaushikMalapati
KaushikMalapati reviewed May 6, 2025
View reviewed changes
KaushikMalapati
KaushikMalapati reviewed May 6, 2025
View reviewed changes
KaushikMalapati
KaushikMalapati reviewed May 6, 2025
View reviewed changes
KaushikMalapati
KaushikMalapati reviewed May 6, 2025
View reviewed changes
@silkenelson @KaushikMalapati
Update scripts/get_info.py
53d5b43 
Co-authored-by: KaushikMalapati <80156796+KaushikMalapati@users.noreply.github.com>
KaushikMalapati
KaushikMalapati reviewed May 6, 2025
View reviewed changes
KaushikMalapati
KaushikMalapati reviewed May 6, 2025
View reviewed changes
@silkenelson @KaushikMalapati
Update scripts/takepeds
449a5f4 
Co-authored-by: KaushikMalapati <80156796+KaushikMalapati@users.noreply.github.com>
KaushikMalapati
KaushikMalapati reviewed May 6, 2025
View reviewed changes
KaushikMalapati
KaushikMalapati reviewed May 6, 2025
View reviewed changes
KaushikMalapati
KaushikMalapati reviewed May 6, 2025
View reviewed changes
KaushikMalapati
KaushikMalapati reviewed May 6, 2025
View reviewed changes
KaushikMalapati
KaushikMalapati reviewed May 6, 2025
View reviewed changes
@KaushikMalapati
Copy link
Contributor

KaushikMalapati commented May 6, 2025

Once you look at all the comments, would you mind running these scripts again and making sure they work?

silkenelson and others added 5 commits May 6, 2025 08:25
@silkenelson @KaushikMalapati
Update scripts/makepeds_psana2
f345629 
Co-authored-by: KaushikMalapati <80156796+KaushikMalapati@users.noreply.github.com>
@silkenelson @KaushikMalapati
Update scripts/makepeds_psana2
b84a246 
Co-authored-by: KaushikMalapati <80156796+KaushikMalapati@users.noreply.github.com>
@silkenelson @KaushikMalapati
Update scripts/get_info.py
d7ec510 
Remove debug prints

Co-authored-by: KaushikMalapati <80156796+KaushikMalapati@users.noreply.github.com>
@silkenelson @KaushikMalapati
Update scripts/makepeds_psana2
def9d0c 
Co-authored-by: KaushikMalapati <80156796+KaushikMalapati@users.noreply.github.com>
@silkenelson
Update makepeds_psana2
b8c4f6c 
fixed & a little more comments.
@tangkong
Copy link
Contributor

tangkong commented May 6, 2025

There are a lot of comments that defer fixes to different efforts. If we're not going to address them immediately, can we make issues for them so we don't lose track of them?

@KaushikMalapati KaushikMalapati mentioned this pull request May 6, 2025
Open
@KaushikMalapati
Copy link
Contributor

KaushikMalapati commented May 6, 2025

There are a lot of comments that defer fixes to different efforts. If we're not going to address them immediately, can we make issues for them so we don't lose track of them?

I only found two which I made issues for (the function for getting the experiment, and putting check_for_status in a try/except). Were there any others?

@KaushikMalapati
Copy link
Contributor

KaushikMalapati commented May 6, 2025 •
edited
Loading

@silkenelson the trailing whitespace, eof, flake8, and isort jobs are failing. Can you commit the changes for those those? I think we can ignore the shellcheck errors for now.

@ZLLentz
Copy link
Member

ZLLentz commented May 6, 2025

fwiw I don't think any PR creator is obligated to spend time solving pre-existing problems unrelated to the PR, maybe we can just run ruff/precommit on this afterwards (see the dormant #270)

@KaushikMalapati
Copy link
Contributor

KaushikMalapati commented May 6, 2025

fwiw I don't think any PR creator is obligated to spend time solving pre-existing problems unrelated to the PR, maybe we can just run ruff/precommit on this afterwards (see the dormant #270)

Makes sense, I was assuming they were introduced here.

@ZLLentz
Copy link
Member

ZLLentz commented May 6, 2025

I guess it's possible some of them are, I didn't look so closely but I know many of them are not created here

@tangkong
Copy link
Contributor

tangkong commented May 6, 2025

I only found two which I made issues for (the function for getting the experiment, and putting check_for_status in a try/except). Were there any others?

Maybe two issues was too much for my goldfish memory. Looking through the comments this looks appropriate

ZLLentz
ZLLentz approved these changes May 6, 2025
View reviewed changes
Copy link
Member

@ZLLentz ZLLentz left a comment

Choose a reason for hiding this comment

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

I think everything here is good to merge
I do want a couple eyes on #270 so we can auto fix some of the incidentals

@silkenelson silkenelson merged commit f836202 into pcdshub:master May 6, 2025
1 of 2 checks passed
KaushikMalapati
KaushikMalapati reviewed May 6, 2025
View reviewed changes
scripts/get_info.py
else:
# then ask.....outside of python
print('unknown_hutch')
sys.exit()
Copy link
Contributor

@KaushikMalapati KaushikMalapati May 6, 2025

Choose a reason for hiding this comment

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

Doesn't this sys.exit() need to be in the else block so that we don't return if we get the hutch from args.setExp?

Copy link
Collaborator Author

@silkenelson silkenelson May 6, 2025

Choose a reason for hiding this comment

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

Shoot, yes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tangkong tangkong tangkong left review comments

@KaushikMalapati KaushikMalapati KaushikMalapati left review comments

@patoppermann patoppermann patoppermann left review comments

@ZLLentz ZLLentz ZLLentz approved these changes

Assignees

@patoppermann patoppermann

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@silkenelson @KaushikMalapati @tangkong @ZLLentz @patoppermann