Skip to content

Fix auxiliary scan ordering - #170

Open
jacob-williamson wants to merge 20 commits into
mainfrom
164_add_order_test
Open

jacob-williamson wants to merge 20 commits into
mainfrom
164_add_order_test

Conversation

@jacob-williamson

@jacob-williamson jacob-williamson commented Nov 9, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #164
Auxiliary scans can now themselves have required auxiliary scans. This allows us to ensure auxiliary scans are ran in the order air -> empty capillary -> standard sample, and also keep the timing benefits of combining auxiliary scans where possible. This is also true in tiled - each auxiliary scan will have links to its own auxiliary scans.

However, it makes things more complicated when looking for a suitable scan in tiled, because a standard sample scan in tiled is only suitable if the empty capillary scan that ran before it is also suitable to the experiment at hand. For this reason, we add the tiled scans own auxiliary scans to the requires field when reconstructing the tiled auxiliary object, which allows the .is_suitable method to verify all the requirements match.

There's quite a lot of recursion here but I can't think of a way to simplify things

@jacob-williamson
jacob-williamson marked this pull request as ready for review November 9, 2026 18:06
@codecov

codecov Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.68%. Comparing base (65a337d) to head (325762e).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #170   +/-   ##
=======================================
  Coverage   98.67%   98.68%           
=======================================
  Files          23       23           
  Lines        1286     1289    +3     
=======================================
+ Hits         1269     1272    +3     
  Misses         17       17           

☔ View full report in Codecov by Harness.
📢 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.

@jacob-williamson jacob-williamson changed the title Fix auxiliary scna ordering Fix auxiliary scan ordering Nov 9, 2026
requires=[air, empty_capillary],
)

return [standard_sample, empty_capillary, air]

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We only really need to have a requirement of standard_sample now, as the others are nested inside it. Should we do that? Similarly StandardSample only needs to depend on empty_capillary. The advantage of having all of them is that tiled links will be added.

@DominicOram DominicOram left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is getting really quite complicated now. Can we have some proper docs on how we're doing auxillary collections in general? I think then it will be easier for me to review

This branch has not been deployed

No deployments
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.

I15-1: Ensure backgrounds are always added in order

2 participants