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

Lens4escan #1202

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Lens4escan #1202

wants to merge 6 commits into from

Conversation

vespos
Copy link
Contributor

@vespos vespos commented Feb 27, 2024

Description

Make lens stack class usable for energy scans with the CCM

Motivation and Context

This includes:

  • Energy functionality based on either user input, mono energy or LCLS energy.
  • Utility function to get the current setup.
  • Report status on move. Needed for scans
  • Allow for either the pulse-picker or attenuator to be used for blocking the beam. Pulse-picker is fast to insert/remove so there is some time to be gained there. Kinda messy, but it works
  • Fix the lens_set vs lens path argument. Do that better?

How Has This Been Tested?

Where Has This Been Documented?

Pre-merge checklist

  • Code works interactively
  • Code contains descriptive docstrings, including context and API
  • New/changed functions and methods are covered in the test suite where possible
  • Test suite passes locally
  • Test suite passes on GitHub Actions
  • Ran docs/pre-release-notes.sh and created a pre-release documentation page
  • Pre-release docs include context, functional descriptions, and contributors as appropriate

DO NOT FORGET TO REMOVE https://github.com/pcdshub/pcdsdevices/pull/1202/files#diff-5a91e8b8e2d4fabd6201376dd1c45357ec3ccf379d64d91641b430e7cad6b84aR222 BEFORE MERGE

z_pos = (dist - self.z_offset) * self.z_dir * 1000
z_pos = z_pos - 100 # dirty trick to get z in the range
Copy link
Contributor Author

Choose a reason for hiding this comment

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

DO NOT FORGET TO REMOVE BEFORE MERGE

@@ -186,11 +213,13 @@ def forward(self, pseudo_pos):
AttributeError
If pseudo motor is not setup for use.
"""
if not np.isclose(pseudo_pos.beam_size, self.beam_size.position):
# if not np.isclose(pseudo_pos.beam_size, self.beam_size.position):
if True:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to self: to check again

@tangkong
Copy link
Contributor

Are we looking to merge this in addition to #1194 ? It seems like there's some duplicated work here

@vespos
Copy link
Contributor Author

vespos commented Feb 28, 2024

Yes, I expect #1194 to be merged first. This is sort of the next step and will likely take a bit more time.
In the meantime I need that other PR code in here for now. I expect git to take care of making this nice once the other PR is merged and I merge master back into this one.

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