This repository was archived by the owner on Sep 2, 2024. It is now read-only.
Rotation scan improvements from testing#915
Merged
Merged
Conversation
- move to the limit if normal nudge would take it over since into-view steps are large - update tests with limits
Codecov Report
@@ Coverage Diff @@
## main DiamondLightSource/hyperion#915 +/- ##
==========================================
+ Coverage 93.32% 93.55% +0.22%
==========================================
Files 53 54 +1
Lines 2562 2590 +28
==========================================
+ Hits 2391 2423 +32
+ Misses 171 167 -4
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
DominicOram
suggested changes
Oct 10, 2023
DominicOram
left a comment
DominicOram
left a comment
Collaborator
There was a problem hiding this comment.
Thanks, some comments in code. But otherwise works well
| # TODO: Check topup gate | ||
| hyperion.log.LOGGER.info("Setting fgs params") | ||
| yield from set_flyscan_params(fgs_motors, parameters.experiment_params) | ||
| hyperion.log.LOGGER.info("Set params") |
Collaborator
There was a problem hiding this comment.
Should: I think this log is overkill
| def append_to_comment(self, comment: str): | ||
| self._append_to_comment(self.ispyb_ids[0], comment) | ||
|
|
||
| def start(self, doc: dict): |
Collaborator
There was a problem hiding this comment.
Should: I think a test on this would be good
| def start(self, doc: dict): | ||
| LOGGER.info("Zocalo handler received start document.") | ||
| if doc.get("subplan_name") == "rotation_scan_main": | ||
| if self.run_uid is None: |
Collaborator
There was a problem hiding this comment.
Must: This is all about making sure the isypb deposition happens before zocalo triggers, right? I think we need a very explicit test that confirms this, otherwise we'll end up accidentally hitting it again
Comment on lines
+37
to
+118
| # All MX collection fields: | ||
| # [ | ||
| # "id", | ||
| # "parentid", | ||
| # "visitid", | ||
| # "sampleid", | ||
| # "detectorid", | ||
| # "positionid", | ||
| # "apertureid", | ||
| # "datacollectionnumber", | ||
| # "starttime", | ||
| # "endtime", | ||
| # "runstatus", | ||
| # "axisstart", | ||
| # "axisend", | ||
| # "axisrange", | ||
| # "overlap", | ||
| # "nimages", | ||
| # "startimagenumber", | ||
| # "npasses", | ||
| # "exptime", | ||
| # "imgdir", | ||
| # "imgprefix", | ||
| # "imgsuffix", | ||
| # "imgcontainersubpath", | ||
| # "filetemplate", | ||
| # "wavelength", | ||
| # "resolution", | ||
| # "detectordistance", | ||
| # "xbeam", | ||
| # "ybeam", | ||
| # "comments", | ||
| # "slitgapvertical", | ||
| # "slitgaphorizontal", | ||
| # "transmission", | ||
| # "synchrotronmode", | ||
| # "xtalsnapshot1", | ||
| # "xtalsnapshot2", | ||
| # "xtalsnapshot3", | ||
| # "xtalsnapshot4", | ||
| # "rotationaxis", | ||
| # "phistart", | ||
| # "kappastart", | ||
| # "omegastart", | ||
| # "resolutionatcorner", | ||
| # "detector2theta", | ||
| # "undulatorgap1", | ||
| # "undulatorgap2", | ||
| # "undulatorgap3", | ||
| # "beamsizeatsamplex", | ||
| # "beamsizeatsampley", | ||
| # "avgtemperature", | ||
| # "actualcenteringposition", | ||
| # "beamshape", | ||
| # "focalspotsizeatsamplex", | ||
| # "focalspotsizeatsampley", | ||
| # "polarisation", | ||
| # "flux", | ||
| # "processeddatafile", | ||
| # "datfile", | ||
| # "magnification", | ||
| # "totalabsorbeddose", | ||
| # "binning", | ||
| # "particlediameter", | ||
| # "boxsizectf", | ||
| # "minresolution", | ||
| # "mindefocus", | ||
| # "maxdefocus", | ||
| # "defocusstepsize", | ||
| # "amountastigmatism", | ||
| # "extractsize", | ||
| # "bgradius", | ||
| # "voltage", | ||
| # "objaperture", | ||
| # "c1aperture", | ||
| # "c2aperture", | ||
| # "c3aperture", | ||
| # "c1lens", | ||
| # "c2lens", | ||
| # "c3lens", | ||
| # ] | ||
| # |
Collaborator
There was a problem hiding this comment.
Must: Remove this please
Comment on lines
+9
to
+20
| def test_ev_to_a_converter(): | ||
| for i in range(len(test_energies)): | ||
| assert convert_eV_to_angstrom(test_energies[i]) == pytest.approx( | ||
| test_wavelengths[i] | ||
| ) | ||
|
|
||
|
|
||
| def test_a_to_ev_converter(): | ||
| for i in range(len(test_wavelengths)): | ||
| assert convert_angstrom_to_eV(test_wavelengths[i]) == pytest.approx( | ||
| test_energies[i] | ||
| ) |
Collaborator
There was a problem hiding this comment.
Nit: It's nicer to use paramterize as it's more obvious which one has failed if only one has
DominicOram
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #905 and fixes #851
Mostly changes made on the beamline as part of testing, of which most are ISPyB deposition fixes and one hack of OAV snapshot paths - issue to fix here (DiamondLightSource/mx-bluesky#328)
Also includes no longer adding wavelength as external parameter and instead calculating it from energy as I didn't want to find it in GDA to send...
To test: