Skip to content

Xiaoya refactor block - #76

Merged
dylanmcreynolds merged 8 commits into
mlexchange:mainfrom
xiaoyachong:xiaoya-refactor-block
Apr 21, 2026
Merged

Xiaoya refactor block#76
dylanmcreynolds merged 8 commits into
mlexchange:mainfrom
xiaoyachong:xiaoya-refactor-block

Conversation

@xiaoyachong

@xiaoyachong xiaoyachong commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

I refactored the LSE code to follow the arroyopy block format. The idea is to use a xx.yaml file to define the operator, listener, and publisher, so we can remove app_xps.py/app.py and start the server using arroyo run xx.yaml. I tested this with splash_timpix simulator and arroyoXPS on my end, and it works well.
image

A few issues for this PR:

  1. For writing vector results to Tiled (TiledResultsPublisher), I previously use rewrite immediately. I change it to append_partition, similar as patch used in XPSTiledLocalImagePublisher. Here is the reference code: append a table/patch an array.

  2. I also opened a companion PR for arroyopy, because it does not call start() in the publisher. This causes an issue for LSEWSResultPublisher, since it connects to the WebSocket listener (LSE) in its start() method.

  3. I didn't move arroyo_reduction out of LSE. If we move it into both arroyosas and arroyoxps, we would end up with duplicated copies in each repo, which may not be ideal. A better solution might be to create a separate repository (e.g., ArroyoLSE) in the future.

  4. There is a mismatch between the scan names sent from LabVIEW (temp name {uuid.uuid4()}) and Timepix (acquisition_YYYYMMDDTHHMMSSZ_<uuid8>). To make the WebSocket listener work for both formats, I added a temporary fix: if a 36-character UUID is present in scan_name, use it directly; otherwise, generate a new one. A future improvement would be to update splash_timepix to send uuid.uuid4() as the scan name. This is necessary because previous SAS and XPS LabVIEW experiments all use uuid.uuid4() as the scan name in the Tiled URL.

Companion PRs:
arroyoXPS: als-computing/ArroyoXPS#43
arroyopy: als-computing/arroyopy#18

@dylanmcreynolds

Copy link
Copy Markdown
Member

For writing vector results to Tiled (TiledResultsPublisher), I’m currently using rewrite immediately. The correct approach should be patch, as used in XPSTiledLocalImagePublisher. However, I found that array patch exists in tiled:0.1.0-b13, but not table frame patch. I’m not sure which Tiled version to use to support something like patch_dataframe.

We really should bump tiled up to 0.2.8 or greater to be compatible with modern tiled.

@dylanmcreynolds dylanmcreynolds left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In pyproject.toml, what would you think of adding tiled[client]==0.2.8 to the arroyo dependency list? Then we could more surely know what tiled we have, rather than have it sneak in from arroyoysas

@dylanmcreynolds dylanmcreynolds left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it the settings.yml file in the root still needed anymore?

Comment thread Dockerfile_arroyo Outdated

@taxe10 taxe10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Changes look good. At a high-level, do we still need to make a distinction between scattering and XPS beyond the configuration files? Would it be possible to maintain a single LSE operator regardless of the technique (assuming that the input is a 2D frame ofc)?

Comment thread block_configs/lse_sas_operator_block.yaml
Comment thread pyproject.toml Outdated
@xiaoyachong

xiaoyachong commented Apr 17, 2026

Copy link
Copy Markdown
Contributor Author

it the settings.yml file in the root still needed anymore?

Nope. I just remove it. And I also remove app.py and app_xps.py.

@xiaoyachong

xiaoyachong commented Apr 17, 2026

Copy link
Copy Markdown
Contributor Author

Changes look good. At a high-level, do we still need to make a distinction between scattering and XPS beyond the configuration files? Would it be possible to maintain a single LSE operator regardless of the technique (assuming that the input is a 2D frame ofc)?

Thanks for your review. At the moment, we need to distinguish between SAS and XPS because the listeners and publishers are different.

For SAS (e.g., at NSLS-II), we use a LatentSpaceOperator to receive RawFrameEvent, which already contains the image and the Tiled URL. The image does not need to be written to Tiled, since NSLS-II handles that and publishes it through the WebSocket server. The arroyosas WebSocket listener processes it and sends RawFrameEvent to the LatentSpaceOperator.

For Timepix/LabVIEW XPS experiments, arroyoxps sends WebSocket messages that include only the image. The arroyo_reduction then uses an XPSWebSocketListener to receive the message and convert it into a RawFrameEvent for the LatentSpaceOperator. In this case, we also need an XPSTiledLocalImagePublisher to write the image to Tiled, which is not required for SAS.

Because of this, the workflows are different. I’m wondering what Dylan’s @dylanmcreynolds plan is for refactoring this in the long term.

@taxe10

taxe10 commented Apr 17, 2026

Copy link
Copy Markdown
Member

Thanks for clarifying @xiaoyachong

If I understood correctly, if arroyoxps were to write to Tiled and send both the image and the Tiled URL, then in principle we could unify this into a single LSE operator, since the downstream interface (RawFrameEvent) would be consistent.

If that’s not feasible at this stage, I wonder if it might be worth moving away from naming these paths based on experimental techniques (SAS vs XPS). The current distinction seems to stem more from differences in the data ingestion/publishing pipeline (e.g., whether the image is already persisted in Tiled and how events are constructed) rather than the technique itself.

Framing this in terms of the publisher/listener behavior might make the abstraction a bit clearer and more extensible long-term.

Also curious what @dylanmcreynolds thoughts are regarding this. Follow-up comment below - there may be a confusion

@taxe10

taxe10 commented Apr 17, 2026

Copy link
Copy Markdown
Member

Actually - just to clarify my previous comment a bit further. I saw there was an app.py and app_xps.py - that's what I mean by distinction between scattering and XPS beyond the configuration files - so now I am a bit confused because they have been removed. Is the config.yml the only distinction between the 2 now? because if this is the case, this is perfect

@xiaoyachong

Copy link
Copy Markdown
Contributor Author

For writing vector results to Tiled (TiledResultsPublisher), I’m currently using rewrite immediately. The correct approach should be patch, as used in XPSTiledLocalImagePublisher. However, I found that array patch exists in tiled:0.1.0-b13, but not table frame patch. I’m not sure which Tiled version to use to support something like patch_dataframe.

We really should bump tiled up to 0.2.8 or greater to be compatible with modern tiled.

Sure. I just upgrade Tiled to 0.2.8.

@xiaoyachong

Copy link
Copy Markdown
Contributor Author

In pyproject.toml, what would you think of adding tiled[client]==0.2.8 to the arroyo dependency list? Then we could more surely know what tiled we have, rather than have it sneak in from arroyoysas

Sure. I add it to arroyo dependency list.

@xiaoyachong

Copy link
Copy Markdown
Contributor Author

Actually - just to clarify my previous comment a bit further. I saw there was an app.py and app_xps.py - that's what I mean by distinction between scattering and XPS beyond the configuration files - so now I am a bit confused because they have been removed. Is the config.yml the only distinction between the 2 now? because if this is the case, this is perfect

Yes, I removed the unused settings.yaml, app.py, and app_xps.py. The two YAML files in block_configs are now the only distinction between SAS and XPS experiments.

It seems I misunderstood your message earlier—I thought you were referring to the workflows between SAS and XPS being different.

@xiaoyachong

Copy link
Copy Markdown
Contributor Author

Thanks for clarifying @xiaoyachong

If I understood correctly, if arroyoxps were to write to Tiled and send both the image and the Tiled URL, then in principle we could unify this into a single LSE operator, since the downstream interface (RawFrameEvent) would be consistent.

If that’s not feasible at this stage, I wonder if it might be worth moving away from naming these paths based on experimental techniques (SAS vs XPS). The current distinction seems to stem more from differences in the data ingestion/publishing pipeline (e.g., whether the image is already persisted in Tiled and how events are constructed) rather than the technique itself.

Framing this in terms of the publisher/listener behavior might make the abstraction a bit clearer and more extensible long-term.

Also curious what @dylanmcreynolds thoughts are regarding this. Follow-up comment below - there may be a confusion

Thanks for your suggestion. I think moving the Tiled publisher (for saving images to Tiled and generating a Tiled URL) into ArroyoXPS makes sense. I recall that Dylan has plans to refactor the entire pipeline, so we could address this in a future PR.

@dylanmcreynolds dylanmcreynolds left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If I understood correctly, if arroyoxps were to write to Tiled and send both the image and the Tiled URL, then in principle we could unify this into a single LSE operator, since the downstream interface (RawFrameEvent) would be consistent.

Tanny, this is feasible in the timepix case at XPS, but not the bassler case. splash_timepix code is creating the heatmap, while arroyXPS has a step that creates it for bassler. This is being a little pedantic though, since I think we can now assume timeepix. So, I love your suggestion.

@xiaoyachong

Copy link
Copy Markdown
Contributor Author

If I understood correctly, if arroyoxps were to write to Tiled and send both the image and the Tiled URL, then in principle we could unify this into a single LSE operator, since the downstream interface (RawFrameEvent) would be consistent.

Tanny, this is feasible in the timepix case at XPS, but not the bassler case. splash_timepix code is creating the heatmap, while arroyXPS has a step that creates it for bassler. This is being a little pedantic though, since I think we can now assume timeepix. So, I love your suggestion.

Shall we move the ArroyoXPS Tiled-writing refactor to a future PR and merge this one first?

At the moment, we write averaged heatmaps in the current structure.

{tiled_base_uri}/{tiled_prefix}/
│
└── {root_segments}/
    └── {YYYY}/                    
        └── {MM}/                 
            └── {DD}/             
                └── {experiment_name}/
                    └── {UUID}/
                        ├── feature_vectors (table for saving feature vector results)
                        └── xps_averaged_heatmaps (3D array for saving heatmaps)

Once Tiled writing is moved into ArroyoXPS, the heatmaps may no longer be saved under the experiment folder. That would require additional changes to how Tiled data is read in both LSE live mode and offline mode (experiment replay).

@dylanmcreynolds
dylanmcreynolds merged commit e880717 into mlexchange:main Apr 21, 2026
2 checks passed
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.

3 participants