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

Prepare ZocaloResults to use multiple results sources #559

Open
dperl-dls opened this issue May 20, 2024 · 7 comments · May be fixed by #763, DiamondLightSource/mx-bluesky#445 or DiamondLightSource/hyperion#1548
Assignees

Comments

@dperl-dls
Copy link
Collaborator

I03 will soon have both GPU-based and traditional xray-centring results available. ZocaloResults should fetch both of these from the queue and pick which one to use based on input from hyperion.

@DominicOram
Copy link
Contributor

Need to discuss with Nick on multiple messages vs single messages with both results

@DominicOram
Copy link
Contributor

@ndevenish says it will be multiple messages. For implementing this lets assume the format is:

{
    "results": [
        {
            "centre_of_mass": [
                2.207133058984911,
                1.4175240054869684,
                13.317215363511659,
            ],
            "max_voxel": [2, 1, 13],
            "max_count": 702.0,
            "n_voxels": 12,
            "total_count": 5832.0,
            "bounding_box": [[1, 0, 12], [4, 3, 15]],
        }
    ],
    "status": "success",
    "type": "3d",
    "source": "gpu",
}

It'll be close to that I suspect. Nick, feel free to correct us if you have a better idea of the format.

@olliesilvester olliesilvester self-assigned this Aug 29, 2024
@ndevenish
Copy link

You are listening to messages directly, right? Are you getting the messages back from the xrc.{ispyb_beamline} queue?

It might be in the message parameters, along with dcid/dcgid, is that okay?

@olliesilvester
Copy link
Collaborator

As discussed with @DominicOram:

  • We want a parameter to turn on GPU processing and a separate one for using the GPU results
  • If use_gpu_results is on, then always use the first message we recieve from ZocaloResults. Otherwise, use the CPU results

@olliesilvester
Copy link
Collaborator

olliesilvester commented Aug 29, 2024

As part of this issue: if we are using gpu results, we would like to compare data from GPU and CPU results and include it in a debug log, and warn if the results differ by some tolerance (TBC)

This should be done asynchronously so we aren't still being blocked by slow CPU results

@olliesilvester
Copy link
Collaborator

olliesilvester commented Aug 30, 2024

Updated acceptance criteria:

  • Add parameter to use zocalo gpu results
  • If this is true, then wait for zocalo results from both cpu and gpu. Warn if the CPU results arrived first, compare results from CPU and GPU, and warn if they are different at all (within a floating point rounding error).
  • If waiting for zocalo results timeout and we only received one set of results, then warn and use those results
  • If use zocalo_gpu_results is false then just use CPU results as before
  • Create follow up ticket: After we're confident that GPU results are correct, change behaviour so that we no longer wait for both results to arrive and instead only wait for the first result

@DominicOram
Copy link
Contributor

Changes are currently on the beamline to use the CPU results no matter what. Update this accordingly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment