Skip to content

perf: Cytiva Biacore T200 Evaluation Module - Added optimizations on decoding logic - #1109

Merged
slopez-b merged 4 commits into
Benchling-Open-Source:mainfrom
felipenarv:felipenarv/optimizations_for_cytiva_biacore_t200_evaluation
Oct 30, 2025
Merged

perf: Cytiva Biacore T200 Evaluation Module - Added optimizations on decoding logic#1109
slopez-b merged 4 commits into
Benchling-Open-Source:mainfrom
felipenarv:felipenarv/optimizations_for_cytiva_biacore_t200_evaluation

Conversation

@felipenarv

@felipenarv felipenarv commented Oct 24, 2025

Copy link
Copy Markdown
Contributor

The improvement of this update is aprox 37%. This was calculated by running a script that switches from the existing code to the code on the HEAD. The performance below is -58% because it is taking this branch as the base and the code in main as the "new" code. The real improvement is like 37%, less than the 50% I had because I was just timing the code in the parser, this runs the entire test.

This also add a t-test to be sure the improvement is statistically relevant


Benchmark results (lower is better):
- run 1: current_main: 19.718s [ok]
- run 1: baseline_git_HEAD: 12.646s [ok]
- run 2: current_main: 19.186s [ok]
- run 2: baseline_git_HEAD: 12.483s [ok]
- run 3: baseline_git_HEAD: 12.056s [ok]
- run 3: current_main: 19.774s [ok]
- run 4: current_main: 19.427s [ok]
- run 4: baseline_git_HEAD: 12.327s [ok]
- run 5: current_main: 19.672s [ok]
- run 5: baseline_git_HEAD: 12.371s [ok]
Average baseline_git_HEAD: 12.377s ± 0.217s (n=5)
Average current_main: 19.555s ± 0.245s (n=5)
Improvement: -58.0%
Welch t-test: t=-48.979, dof≈7.9
Improvement (baseline - current_main) 95% CI: [-7.420, -6.920] s; P(current_main faster)≈0.000

@james-leinas

Copy link
Copy Markdown
Collaborator

@joshua-benchling and @slopez-b would you be able to give an initial review of Felipe's PR here? Thanks!

@joshua-benchling joshua-benchling left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great performance key improvements, left a question.
Also if you can provide a description on how much the performance was increased would be good.

@felipenarv

Copy link
Copy Markdown
Contributor Author

@joshua-benchling I added some explanations of what was done and why it works. As for the general performance, this is the result of a script that runs a test with the current code in main and this. The performance is -58% because it is taking this branch as the base and the code in main as the "new" code. The real improvement is like 37%, less than the 50% I had because I was just timing the code in the parser, this runs the entire test


Benchmark results (lower is better):
- run 1: current_main: 19.718s [ok]
- run 1: baseline_git_HEAD: 12.646s [ok]
- run 2: current_main: 19.186s [ok]
- run 2: baseline_git_HEAD: 12.483s [ok]
- run 3: baseline_git_HEAD: 12.056s [ok]
- run 3: current_main: 19.774s [ok]
- run 4: current_main: 19.427s [ok]
- run 4: baseline_git_HEAD: 12.327s [ok]
- run 5: current_main: 19.672s [ok]
- run 5: baseline_git_HEAD: 12.371s [ok]
Average baseline_git_HEAD: 12.377s ± 0.217s (n=5)
Average current_main: 19.555s ± 0.245s (n=5)
Improvement: -58.0%
Welch t-test: t=-48.979, dof≈7.9
Improvement (baseline - current_main) 95% CI: [-7.420, -6.920] s; P(current_main faster)≈0.000

@felipenarv felipenarv left a comment

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.

Added some comments explaining why the improvements should work

…acore_t200_evaluation_decoder.py

Co-authored-by: joshua-benchling <jherna@benchling.com>
Signed-off-by: felipenarv <felipe.narvaez@benchling.com>
@slopez-b
slopez-b merged commit 92efd86 into Benchling-Open-Source:main Oct 30, 2025
7 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.

4 participants