-
Notifications
You must be signed in to change notification settings - Fork 0
Centre of mass callback #218
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
Conversation
There was a problem hiding this 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>
what he said fr |
| ``` | ||
| 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 |
There was a problem hiding this comment.
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!)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
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):