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

Beam definer #872

Closed
wants to merge 7 commits into from
Closed

Beam definer #872

wants to merge 7 commits into from

Conversation

meguiraun
Copy link
Contributor

working with beam finer elements:

  • New beam definer mockup
  • Beam info mockup uses this new beam definer

@meguiraun meguiraun changed the title WIP: Beam definer Beam definer Mar 22, 2024
@marcus-oscarsson
Copy link
Member

@meguiraun: Are you ready with this one ?

@@ -0,0 +1,97 @@
# encoding: utf-8
Copy link
Member

Choose a reason for hiding this comment

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

Nice with a mockup 👍

try:
_ap = eval(self.get_property("styling"))
except:
print("malformed xml")
Copy link
Member

Choose a reason for hiding this comment

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

Nicer with a log :)

_val = self.get_value()
self.emit("valueChanged", _val)

def get_custom_styling(self):
Copy link
Member

Choose a reason for hiding this comment

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

What does custom styling do ? :)

self.beam_size_hor = None
self.beam_size_ver = None
self.custom_styling = None
def init(self):
Copy link
Member

Choose a reason for hiding this comment

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

I'm surprised that this does not create a linting error, if not I think it looks better with a blank line between these methods

try:
hor = int(self.beam_size_hor.get_value())
ver = int(self.beam_size_ver.get_value())
_val = f"{hor}x{ver}"
Copy link
Member

Choose a reason for hiding this comment

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

I guess you could do return self.value_to_enum(f"{hor}x{ver}")

@marcus-oscarsson
Copy link
Member

Very nice initiate :) 👍 I made some minor comments you might want to have a look at


self.beam_size_hor = self.get_object_by_role("beam_size_hor")
self.beam_size_ver = self.get_object_by_role("beam_size_ver")
self.beam_size_hor.connect
Copy link
Collaborator

Choose a reason for hiding this comment

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

This line looks really strange.

Copy link
Collaborator

@rhfogh rhfogh left a comment

Choose a reason for hiding this comment

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

Looks nice (to a quick glance)

Comment on lines +179 to +196
if self.beam_size_determination is None:
# we keep the minumum
size_x = min(
self._beam_size_dict["aperture"][0],
self._beam_size_dict["slits"][0],
self._beam_size_dict["definer"][0],
)
size_y = min(
self._beam_size_dict["aperture"][1],
self._beam_size_dict["slits"][1],
self._beam_size_dict["definer"][1],
)
elif self.beam_size_determination == "definer":
size_x = self._beam_size_dict["definer"][0]
size_y = self._beam_size_dict["definer"][1]
elif self.beam_size_determination == "slits":
size_x = self._beam_size_dict["slits"][0]
size_y = self._beam_size_dict["slits"][1]
Copy link
Member

Choose a reason for hiding this comment

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

Could we not just use the already existing beam_definer instead?? Maybe I did not quite get the logic, but if there is a definer, the evaluate_beam should take the definer size. Otherwise, if you still want to continue with your code, you should define aperture as well.

@beteva
Copy link
Member

beteva commented May 23, 2024

@meguiraun I like your idea of introducing a beam definer. Thus we could make an abstraction of what is the hardware, that defines the beam size. This also simplifies the code as there is no more need of the evaluate_beam function, as get_beam_size will give the beam size of whatever the definer is. It is a good idea to provide handling of aperture and slits, but than I believe they should be configured as beam definer.

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.

4 participants