Skip to content

Conversation

@StephenNneji
Copy link
Contributor

@StephenNneji StephenNneji commented Apr 25, 2025

  • Please check if the PR fulfills these requirements
  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • What is the new behaviour (if this is a feature change)?
    This adds the option to download positioner pose (position and orientation) matrices after a simulation. To use the feature:
  1. Run a simulation as normal.
  2. When the simulation is done, click the download button shown in the image below.
  3. Select save location
  4. Each row in the file is a flatten matrix. The matrices are stored column first i.e. [m00, m10, m20, m01, m11, m21, m02, m12, m22, m03, m13, m23] so the first 9 values are for the rotation matrix and the last 3 the translation vector.
  • Does this PR introduce a breaking change (What changes might users need to make in their application due to this PR)?
    No

  • Other information:
    Screenshot 2025-04-29 123451

@andy-bridger
Copy link

Seems to work, if I could be cheeky and request comma separated values in the output file, that would be lovely, but can obviously process the string on our end if needed.

I haven't done I functional test of the matrix being output (although I trust it is correct, I just mean testing it does the same thing in mantid) but that might have to wait until we collect the new experimental data at the end of May (when we have the actual physical reference)

@StephenNneji
Copy link
Contributor Author

Seems to work, if I could be cheeky and request comma separated values in the output file, that would be lovely, but can obviously process the string on our end if needed.

No problem, I will make the output comma separated

@StephenNneji StephenNneji requested a review from RabiyaF May 1, 2025 13:32
Copy link
Member

@RabiyaF RabiyaF left a comment

Choose a reason for hiding this comment

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

I tested it and it works as expected. I just wanted to mention one issue. I can't create new environments for sscanss. I get the following error on M1 and M3 macs.

image

I have some old environments for fitbenchmarking that have Nlopt==2.7.1 installed. I used one of these old conda environments to test the new features of this PR and they work as expected.

Maybe it might be worthwhile to consider updating the numpy dependency to > 2.x.x which will allow installing Nlopt > 2.7.1.

I briefly looked into this. It can be done very easily using ruff. There might be ~13 lines that need to be updated in the codebase. They are the lines that use row_stack.

image

@StephenNneji
Copy link
Contributor Author

Maybe it might be worthwhile to consider updating the numpy dependency to > 2.x.x which will allow installing Nlopt > 2.7.1

I will do another PR for upgrading package versions

@StephenNneji
Copy link
Contributor Author

Seems to work, if I could be cheeky and request comma separated values in the output file, that would be lovely, but can obviously process the string on our end if needed.

@andy-bridger, I had a look at the other exports and tabs are used everywhere, so to keep consistency I will leave the tab separated values as is, but I could revisit in the future maybe by adding a setting to allow the delimiter changes globally.

@StephenNneji StephenNneji merged commit 6b8cd58 into master May 7, 2025
3 checks passed
@StephenNneji StephenNneji deleted the export_pose branch September 3, 2025 13:34
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