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

P11 develop debug #859

Merged
merged 22 commits into from
Mar 7, 2024
Merged

Conversation

agruzinov
Copy link
Contributor

Debug after upgrading to the latest development.

@@ -486,3 +486,19 @@ def _update_selection(self):
s._set_loaded(False)

self._set_selected_sample(None)

def _set_loaded_sample(self, sample):
Copy link
Member

Choose a reason for hiding this comment

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

@agruzinov, is defining this method solves the recurrence problem you've mentioned?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it solves the "loading failed" problem.

Comment on lines +139 to +140
osc_pars["kappa"] = 0
osc_pars["kappa_phi"] = 0
Copy link
Member

Choose a reason for hiding this comment

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

Do you have minikappa head on your diffarctometer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this is just not to have none values, just in case. Does it make sense?

Copy link
Member

Choose a reason for hiding this comment

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

It is OK if you would never use minikappa.

@agruzinov
Copy link
Contributor Author

Funny enough after synchronising with the latest develop on github webpage, linting test is failing now, although I did not change a contenent.

if start_at >= stop_angle:
init_pos = start_at

init_pos = start_at # - self.acq_speed * self.turnback_time
Copy link
Collaborator

Choose a reason for hiding this comment

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

Currently both branches of the 'if' statement are identical - and anyway init_pos is not used so both do nothing. Harmless, but maybe a clean-up or comment-out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did it, thanks! It is still not clear if this is an issue or not.

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.

Fine with me

@agruzinov agruzinov requested a review from beteva March 5, 2024 10:40
@@ -333,7 +333,7 @@ def manual_centring(self, phi_range=120, n_points=3):
self.user_clicked_event = AsyncResult()
x, y = self.user_clicked_event.get()
if click < 2:
phi_mot.set_value_relative(phi_range)
phi_mot.set_value_relative(phi_range, timeout=5)
Copy link
Member

@beteva beteva Mar 6, 2024

Choose a reason for hiding this comment

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

If your phi_mot inherits from AbstractMotor, than only means that you will wait max 5s for the motor to move an will raise TimeoutError if the movement takes longer. It does not give you an extra time for the clicking.
Maybe your problem was that by default the timeout=0, meaning, do not wait for the movement to finish.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the comment! What would be the best way to mitigate it? I do not see any "wait" option there.

Copy link
Member

@beteva beteva Mar 6, 2024

Choose a reason for hiding this comment

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

timeout=None waits forever. But you can as well put some big value, like 2 min (timeout=60). This is only security - the command will return as soon as the movement has finished.

You may want to have a look at the AbstractMotor.py. It says:
timeout (float): optional - timeout [s], If timeout == 0: return at once and do not wait; if timeout is None: wait forever

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, thanks for the tip. Cheers!

Comment on lines +580 to +592
# Keep it here as it is not clear if it is needed.
# It was used in CC to fix the issue with the data processing
# h5fd["entry/sample/transformations/omega"].attrs["vector"] = [
# 1.0,
# 0.0,
# 0.0,
# ]
# h5fd["entry/instrument/detector/module/fast_pixel_direction"].attrs[
# "vector"
# ] = [1.0, 0.0, 0.0]
# h5fd["entry/instrument/detector/module/slow_pixel_direction"].attrs[
# "vector"
# ] = [0.0, 1.0, 0.0]
Copy link
Member

Choose a reason for hiding this comment

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

This represents the direction of the diffractometer head in your metadata image. If your detector is Eiger and you use the dectris filewriter, these values are set by default as horizontal orientation, so you need to change to something else if your diffractometer is not the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

@marcus-oscarsson marcus-oscarsson merged commit c5166d5 into mxcube:develop Mar 7, 2024
8 checks passed
@agruzinov agruzinov deleted the p11_develop_debug branch March 8, 2024 10:17
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