Skip to content

Keep summed chains in one row with their feeders tucked under - #254

Merged
BradyAJohnston merged 2 commits into
mainfrom
layout-feeder-rows
Oct 12, 2026
Merged

BradyAJohnston merged 2 commits into
mainfrom
layout-feeder-rows

Conversation

@BradyAJohnston

Copy link
Copy Markdown
Owner

Chains summed through a node's first input (e.g. the Add chains in MolecularNodes' Composite ID Outline / Cone Shadow, BradyAJohnston/MolecularNodes#1240) were laid out as a staircase, with all the feeders piled into the first columns.

  • placement.py: when a node has two median predecessors, align with the one across the heavier link first, so a Float-only chain stays one row in all four BK runs.
  • feeders.py (new): a node whose links out all go to one node of its frame is moved to the column just before that node (move_feeders_right), placed next to the other node of that column feeding the same node (tuck_feeders), and pulled against it after placement (pull_up_feeders).

Corpus snapshots updated; new summed_samples case and tests for it and value_chain. Net corpus cost goes up (more long links, e.g. Scatter on Surface crossings 241 -> 300), and the steps also run under "simple". To review more broadly before merging.

Before (main) above, after below:

Composite ID Outline
Composite Cone Shadow
summed_samples
value_chain

Essentials

Screen to 3D Space
Transform and Project
Random Rotation
Face Corner Angle
Randomize Transforms
half_spheres

Images live on the pr-images/layout-feeder-rows branch; delete it after merging.

@codecov

codecov Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.79518% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 98.31%. Comparing base (3f0481c) to head (fd378ef).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/nodebpy/layout/feeders.py 98.18% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #254      +/-   ##
==========================================
+ Coverage   98.29%   98.31%   +0.01%     
==========================================
  Files         105      106       +1     
  Lines       31104    31168      +64     
==========================================
+ Hits        30575    30642      +67     
+ Misses        529      526       -3     

☔ 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.

@codspeed

codspeed Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will improve performance by 38.14%

⚠️ 1 benchmark spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 3 improved benchmarks
✅ 12 untouched benchmarks

Performance Changes

Benchmark BASE HEAD Efficiency
⚡ test_arrange_decoder_4bit[sugiyama] 5.3 s 3.5 s +51.16%
⚡ test_arrange_math_expression 2.4 s 1.8 s +35.41%
⚡ test_arrange_decoder_4bit[simple] 1.7 s 1.3 s +28.78%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing layout-feeder-rows (fd378ef) with main (3f0481c)1

Open in CodSpeed

Footnotes

  1. No successful run was found on balance-socket-ids (6e78b4d) during the generation of this report, so main (3f0481c) was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

@BradyAJohnston
BradyAJohnston marked this pull request as ready for review October 9, 2026 09:01
Hashing the Socket dataclass was most of the cost of each candidate move. Layouts are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@BradyAJohnston
BradyAJohnston changed the base branch from main to balance-socket-ids October 12, 2026 11:27
Base automatically changed from balance-socket-ids to main October 12, 2026 11:38
@BradyAJohnston
BradyAJohnston merged commit 275c5c2 into main Oct 12, 2026
19 checks passed
@BradyAJohnston
BradyAJohnston deleted the layout-feeder-rows branch October 12, 2026 12:01
BradyAJohnston added a commit that referenced this pull request Oct 12, 2026
* Count sockets by number when balancing column heights

Hashing the Socket dataclass was most of the cost of each candidate move. Layouts are unchanged.

* Keep summed chains in one row with their feeders tucked under
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.

1 participant