Skip to content

feat(pyamber): clean up reassigned channels - #8228

Open
carloea2 wants to merge 1 commit into
apache:mainfrom
carloea2:fix/input-channel-reassignment
Open

feat(pyamber): clean up reassigned channels#8228
carloea2 wants to merge 1 commit into
apache:mainfrom
carloea2:fix/input-channel-reassignment

Conversation

@carloea2

@carloea2 carloea2 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Remove a channel from its previous input port before registering it with another port. This keeps the forward mapping and each port's channel set consistent for port-aligned control messages.

Any related issues, documentation, discussions?

Closes #8227

How was this PR tested?

python -c "import sys,pytest; sys.path[:0]=[worktree amber source, main amber source]; raise SystemExit(pytest.main(['amber/src/test/python/core/architecture/packaging/test_input_manager.py','amber/src/test/python/core/architecture/managers/test_embedded_control_message_manager.py','amber/src/test/python/core/architecture/handlers/control/test_add_input_channel_handler.py','-q','-p','no:cacheprovider']))"
40 passed

ruff check amber/src/main/python amber/src/test/python
All checks passed

ruff format --check amber/src/main/python amber/src/test/python
213 files already formatted

A direct post-fix probe confirmed both mapping directions agree:

mapped_port=1
old_port_contains_channel=False
new_port_contains_channel=True
old_port_channel_count=0

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Codex

@Yicong-Huang Yicong-Huang added the release/v1.2 back porting to release/v1.2 label Aug 31, 2026
@github-actions
github-actions Bot requested a review from xuang7 August 31, 2026 04:07
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Backport auto-label report

This fix: PR was checked against each actively-supported release branch. release/* labels drive the post-merge backport, so add or remove one to change where this fix lands.

Release branch Analysis
⚠️ release/v1.3 Not labeled automatically — none of the files this PR modifies exist on this branch (amber/src/main/python/core/architecture/packaging/input_manager.py, amber/src/test/python/core/architecture/packaging/test_input_manager.py). The fix may target code that isn't on this release, or the files were moved/renamed after the branch was cut. Please check and add release/v1.3 by hand if this fix should be backported here.
release/v1.2 Already labeled — this fix is queued to backport here.

Auto-label run.

@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @aglinxinyuan, @eugenegujing
    You can notify them by mentioning @aglinxinyuan, @eugenegujing in a comment.

@codecov-commenter

codecov-commenter commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.94%. Comparing base (70c2114) to head (e400d5a).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #8228   +/-   ##
=========================================
  Coverage     93.94%   93.94%           
  Complexity     4759     4759           
=========================================
  Files          1185     1185           
  Lines         48069    48072    +3     
  Branches       5359     5359           
=========================================
+ Hits          45159    45162    +3     
  Misses         1480     1480           
  Partials       1430     1430           
Flag Coverage Δ *Carryforward flag
access-control-service 81.00% <ø> (ø) Carriedforward from 70c2114
agent-service 99.32% <ø> (ø) Carriedforward from 70c2114
amber 89.99% <ø> (ø) Carriedforward from 70c2114
computing-unit-managing-service 73.67% <ø> (ø) Carriedforward from 70c2114
config-service 86.86% <ø> (ø) Carriedforward from 70c2114
file-service 87.91% <ø> (ø) Carriedforward from 70c2114
frontend 96.49% <ø> (ø) Carriedforward from 70c2114
notebook-migration-service 79.31% <ø> (ø) Carriedforward from 70c2114
pyamber 98.87% <100.00%> (+<0.01%) ⬆️
workflow-compiling-service 77.19% <ø> (ø) Carriedforward from 70c2114

*This pull request uses carry forward flags. Click here to find out more.

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

🟢 2 better · 🔴 7 worse · ⚪ 6 noise (<±5%) · 0 without baseline

Compared against main 70c2114 benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

config throughput MB/s latency max Δ latest / 7d
🔴 bs=10 sw=10 sl=64 543 0.332 17,913/24,095/24,095 us 🔴 +25.5% / 🔴 +54.7%
🔴 bs=100 sw=10 sl=64 1,209 0.738 79,438/106,672/106,672 us 🔴 +5.4% / 🟢 -21.1%
🔴 bs=1000 sw=10 sl=64 1,378 0.841 728,136/796,193/796,193 us 🔴 +6.6% / 🟢 +32.2%
Baseline details

Latest main 70c2114 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 543 tuples/sec 596 tuples/sec 787.98 tuples/sec -8.9% -31.1%
bs=10 sw=10 sl=64 MB/s 0.332 MB/s 0.364 MB/s 0.481 MB/s -8.8% -31.0%
bs=10 sw=10 sl=64 p50 17,913 us 14,273 us 12,593 us +25.5% +42.2%
bs=10 sw=10 sl=64 p95 24,095 us 28,544 us 15,579 us -15.6% +54.7%
bs=10 sw=10 sl=64 p99 24,095 us 28,544 us 18,786 us -15.6% +28.3%
bs=100 sw=10 sl=64 throughput 1,209 tuples/sec 1,229 tuples/sec 1,008 tuples/sec -1.6% +20.0%
bs=100 sw=10 sl=64 MB/s 0.738 MB/s 0.75 MB/s 0.615 MB/s -1.6% +20.0%
bs=100 sw=10 sl=64 p50 79,438 us 79,848 us 100,701 us -0.5% -21.1%
bs=100 sw=10 sl=64 p95 106,672 us 101,180 us 107,244 us +5.4% -0.5%
bs=100 sw=10 sl=64 p99 106,672 us 101,180 us 116,122 us +5.4% -8.1%
bs=1000 sw=10 sl=64 throughput 1,378 tuples/sec 1,418 tuples/sec 1,042 tuples/sec -2.8% +32.2%
bs=1000 sw=10 sl=64 MB/s 0.841 MB/s 0.866 MB/s 0.636 MB/s -2.9% +32.2%
bs=1000 sw=10 sl=64 p50 728,136 us 705,778 us 981,959 us +3.2% -25.8%
bs=1000 sw=10 sl=64 p95 796,193 us 746,950 us 1,023,080 us +6.6% -22.2%
bs=1000 sw=10 sl=64 p99 796,193 us 746,950 us 1,051,697 us +6.6% -24.3%
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,368.03,200,128000,543,0.332,17913.00,24094.65,24094.65
1,100,10,64,20,1654.35,2000,1280000,1209,0.738,79438.13,106671.63,106671.63
2,1000,10,64,20,14512.80,20000,12800000,1378,0.841,728135.84,796192.72,796192.72

@Yicong-Huang

Copy link
Copy Markdown
Contributor

I am converting this PR to a draft: please follow our PR template and mark it ready for review.

@Yicong-Huang
Yicong-Huang marked this pull request as draft August 31, 2026 20:10
@carloea2
carloea2 marked this pull request as ready for review August 31, 2026 20:18
@carloea2

Copy link
Copy Markdown
Contributor Author

The description now follows the current template, and this is ready for review.

@carloea2 carloea2 changed the title fix(pyamber): clean up reassigned channels feat(pyamber): clean up reassigned channels Aug 31, 2026
@mengw15 mengw15 removed the release/v1.2 back porting to release/v1.2 label Aug 31, 2026
@xuang7 xuang7 removed the fix label Sep 1, 2026
@xuang7
xuang7 removed their request for review September 1, 2026 01:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reassigned input channels remain attached to old ports

5 participants