Skip to content

Conversation

@jackbdoughty
Copy link
Contributor

@jackbdoughty jackbdoughty commented Jun 27, 2025

We could not use Bluesky's centre of mass callback as it was not suited for the way we would want to use it, so this is our implementation for one.

To test:

Make sure that all tests pass.
Run the following scans and check that you when you subscribe CoM callback, you get sensible CoMs for each. (Send me a message if you need to know how to set up to scan blocks against eachother / create fake data):

  • A scan with non constant point spacing.
  • A scan where some of the values are negative / have a background.
  • The "there-and-back" scan so x points would be like: -2 -1 0 1 2 1 0 -1 -2. (Non continuous points)

@jackbdoughty jackbdoughty added the bluesky-Semver-Minor New functionality / back-compatible changes label Jun 27, 2025
@jackbdoughty jackbdoughty marked this pull request as ready for review June 27, 2025 12:42
Copilot AI review requested due to automatic review settings June 27, 2025 12:42
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements a custom CentreOfMass callback to calculate the centre of mass of a 2D area under the curve, replacing Bluesky's implementation for better suitability with disordered and non-continuous data.

  • New CentreOfMass callback and corresponding tests have been added.
  • Updates in the ISISCallbacks class and documentation to integrate the new callback.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/callbacks/test_isis_callbacks.py Adds tests to verify callback setup and error handling for CentreOfMass.
tests/callbacks/fitting/test_centre_of_mass.py Provides comprehensive parameterized tests covering various data cases for CentreOfMass.
src/ibex_bluesky_core/callbacks/_fitting.py Introduces the centre_of_mass_of_area_under_curve function and the CentreOfMass callback class.
src/ibex_bluesky_core/callbacks/init.py Integrates CentreOfMass into the callbacks collection and ISISCallbacks properties.
doc/fitting/fitting.md Updates documentation with a new section for the CentreOfMass callback.
doc/architectural_decisions/007-centre-of-mass.md Documents the design decision to implement a custom CentreOfMass callback.
Comments suppressed due to low confidence (1)

doc/fitting/fitting.md:216

  • The docs refer to a property named 'value' but the implementation uses 'result'. Please update the documentation to match the actual property name.
[`CentreOfMass`](ibex_bluesky_core.callbacks.CentreOfMass) has a property, `value` which stores the centre of mass value once the callback has finished.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@jackbdoughty
Copy link
Contributor Author

Pull Request Overview

This PR implements a custom CentreOfMass callback to calculate the centre of mass of a 2D area under the curve, replacing Bluesky's implementation for better suitability with disordered and non-continuous data.

  • New CentreOfMass callback and corresponding tests have been added.
  • Updates in the ISISCallbacks class and documentation to integrate the new callback.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
Comments suppressed due to low confidence (1)

what he said fr

@jackbdoughty jackbdoughty mentioned this pull request Jun 27, 2025
1 task
```
See the [standard fits](#models) list above for standard fits which require parameters. It gets more complicated if you want to define your own custom model or guess which you want to pass parameters to. You will have to define a function that takes these parameters and returns the model / guess function with the subsituted values.

# Centre of Mass
Copy link
Contributor

Choose a reason for hiding this comment

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

as above i don't think this wants to be in fitting, but i think that the fitting section could do with a rework as it has callbacks in at the moment - we should move them to the callbacks header as with the CentreOfMass cb. Also I need to add #244 to it (probably wants to be the first page!)

Copy link
Member

Choose a reason for hiding this comment

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

I'll refactor fitting to be entirely under the callbacks header I think.

Copy link
Member

Choose a reason for hiding this comment

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

will fix in documentation mega-pr

@rerpha rerpha merged commit fec32f3 into main Jul 22, 2025
13 checks passed
@rerpha rerpha deleted the centre_of_mass_callback branch July 22, 2025 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bluesky-Semver-Minor New functionality / back-compatible changes size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants