Skip to content

Add adapter controller#8

Open
LuisFSegalla wants to merge 14 commits intomainfrom
add-adapter-controller
Open

Add adapter controller#8
LuisFSegalla wants to merge 14 commits intomainfrom
add-adapter-controller

Conversation

@LuisFSegalla
Copy link
Copy Markdown
Contributor

Add a custom FP adapter controller for Xspress.

TODO:

  • add tests

@LuisFSegalla LuisFSegalla force-pushed the add-adapter-controller branch from d031d89 to 424f0b4 Compare March 9, 2026 09:09
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.32%. Comparing base (e4830a5) to head (eafdaee).

Additional details and impacted files
@@            Coverage Diff             @@
##             main       #8      +/-   ##
==========================================
+ Coverage   94.20%   95.32%   +1.12%     
==========================================
  Files           4        5       +1     
  Lines          69      107      +38     
==========================================
+ Hits           65      102      +37     
- Misses          4        5       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread .github/workflows/_pypi.yml Outdated
Comment thread src/fastcs_xspress/xspress_fp_adapter_controller.py Outdated
Comment thread src/fastcs_xspress/xspress_fp_adapter_controller.py
@LuisFSegalla LuisFSegalla requested a review from shihab-dls March 24, 2026 09:20
Copy link
Copy Markdown

@shihab-dls shihab-dls left a comment

Choose a reason for hiding this comment

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

I've left a comment to discuss

Comment on lines +16 to +40
await super().initialise()

# Construct a list with all the MCA dataset chunks
mca_list = []
for sub_controller in get_all_sub_controllers(self):
match sub_controller:
case OdinSubController():
for parameter in sub_controller.parameters:
if "chunks" in parameter.uri and "0" in parameter.uri:
# Parameter name will be in the form of mca_X_chunks_0
# sub_controller path will be in the form of ["FP", "X",...]
mca_list.append(
(sub_controller.path[1], parameter.name.split("_")[1])
)
case _:
logging.warning(
f"Subcontroller {sub_controller} not an OdinAdapterController"
)

async def set_chunk_fp(value: int):
for sub_controller, mca_num in mca_list:
await self.connection.put(
f"api/0.1/fp/{sub_controller}/config/hdf/dataset/mca_{mca_num}/chunks",
[value, 1, 4096], # pyright: ignore[reportArgumentType]
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I have some question to gain the context I need to propery review this. Is the idea that each FP will have sequence of mca.. attributes, and we need to synchronise these accross all FPs? If so, would it be neater to override _subcontroller_cls in XspressFPAdapterController, defining our own XspressFPController which has a top level mca attribute, that, on initialise, finds all mca attributes on itself, and binds an update callback to sync them. So, we would have XspressController which has a child XspressFPAdapterController-> this child will have a top level chunks that fans out to its vector children XspressFPControllers top level mca attribute (i.e., will use self._children to collect a list of the mca attrs)-> these attributes update the individual mca attributes they have. Possibly I am not understanding the structure we want here though!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi Shihab, just giving some more context to continue the discussion:

Xspress is a bit special where it has it's own cpp adapter that implements some features, one of them being a configurable chunking size. This essentially batches a bunch of frames together before sending them down the plugin chain. This configurable chunking feature is exposed here.
To make sure that we get data that makes sense it's necessary to send the same chunking information to the HDF plugin so that it can configure the datasets with the correct shape in order to write the data (those would be the MCA_n datasets that I'm looking for in the fastcs adapter code).
One problem me and Gary noticed when developing this solution initially was that FastCS doesn't create:

dataset_mca_n_chunk

but instead it creates:

dataset_mca_n_chunk_0
dataset_mca_n_chunk_1
dataset_mca_n_chunk_2

This makes it so that when we're initialising the adapter and go through _create_config_fan_attributes those parameter will not be "bundle" together into one attribute that we can write to, leading to the code above where I need to manually look for all the instances where we have a chunk dataset in the N FPs that we might have.
We also had another problem where Odin-data wasn't being the best at handling a single element being passed down to dataset_mca_n_chunk_x which is why we're currently using a connection.putand sending the whole array down.

With all that. I do agree with you that the code would look neater if we could bundle the parameters in a separate overwritten class. If that can be done while respecting the restrictions that we currently have with how xspress-detector is implemented I'm happy to go on with the change. I'll give it a try and push some code for you to review later.

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.

Create Xspress FrameProcessor adapter controller with scalar chunk / batch size

3 participants