Skip to content

146 add support for multi element channel modeling#205

Open
capn-freako wants to merge 12 commits into
masterfrom
146-add-support-for-multi-element-channel-modeling
Open

146 add support for multi element channel modeling#205
capn-freako wants to merge 12 commits into
masterfrom
146-add-support-for-multi-element-channel-modeling

Conversation

@capn-freako

@capn-freako capn-freako commented Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

I turned Claude loose on this issue and I think we've got a workable first-pass solution, after some manual "adjustments" to his fix.

Still to do:

  • Make sure we're actually testing correct s4p concatenation by PyBERT.

capn-freako and others added 7 commits June 16, 2026 08:27
@capn-freako @claude
Add composite interconnect support and fix IBIS-AMI getwave test
bc19c48 
- Expand ch_file trait to ch_files list; add Add/Remove/Clear buttons
  and a read-only queue display so users can build a composite channel
  by cascading multiple S-parameter/time-domain files via skrf **
- Persist ch_files in PyBertCfg (backward-compat: falls back to ch_file
  when list is empty)
- Fix IBISModel() calls: drop stale third positional arg that no longer
  exists in the updated pyibisami signature
- Fix run_ami_model(): use AmiModelResponseKey constants (not plain
  strings) when indexing the dict returned by AMIModel.get_responses()
- Add CLAUDE.md with commands, architecture, and key invariants

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@capn-freako @claude
Update uv.lock
f56b000 
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@capn-freako @claude
Update PyChOpMarg submodule
7d3c66c 
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@capn-freako
Fixed a bug in the path name formatter.
2b78a07
@capn-freako @claude
Absorb channel file browsing into Add button
c130a8e 
The separate FileEditor browse widget is removed; clicking Add now opens
a native pyface FileDialog directly and appends the chosen path to
ch_files in one step.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@capn-freako
GUI rearrangement.
b60a4ea
@capn-freako
Some manual adjustments to Claude's solution.
fc5c947
@capn-freako capn-freako linked an issue Jun 17, 2026 that may be closed by this pull request
Add support for multi-element channel modeling. #146
Open
@capn-freako capn-freako requested a review from jdpatt June 17, 2026 11:26
@jdpatt

jdpatt commented Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Didn't get much time to dig deep but a few changes/comments off the cuff. I don't know if you ever tried my branch where I was removing traits and also added this feature and had sent for you to demo. Deltas from that and features worth while adding:

  • Should use a radio button group to switch widgets between native/files. Much nicer user visual of which is enabled vs the gray out or not. Really this might be nice for all builtin or external features.
  • Add/Remove Last/Clear All could get annoying. I had and propose to add here a Up/Down Arrow or drag handle per Row to reorder rows + a "X" at the end to remove that row. You could have a single Add or Remove all at top then.
  • I think we could drop "Section" from the buttons as that is apparent.
  • I assume fix ports and applying a window or eventual manual port reordering should be per file not applied to all files. Which would ripple into the user configuration object.
  • Let claude add some test coverage of a single file, multiple files and etc.. Use scikit-rf to generate said networks; make sure the fix ports is applying to all/some or a mix.

@capn-freako

capn-freako commented Jun 19, 2026

Copy link
Copy Markdown
Owner Author
  • Should use a radio button group to switch widgets between native/files. Much nicer user visual of which is enabled vs the gray out or not. Really this might be nice for all builtin or external features.

Sorry, do you mean that the user's radio button selection makes the irrelevant half disappear?
(That's how I'm reading, "much nicer than graying it out.")

@capn-freako

capn-freako commented Jun 19, 2026

Copy link
Copy Markdown
Owner Author
  • Add/Remove Last/Clear All could get annoying. I had and propose to add here a Up/Down Arrow or drag handle per Row to reorder rows + a "X" at the end to remove that row. You could have a single Add or Remove all at top then.

Yes! That'd be much nicer.
Do you have time to make this change or shall I sick Claude on it?

@capn-freako

capn-freako commented Jun 19, 2026

Copy link
Copy Markdown
Owner Author
  • I think we could drop "Section" from the buttons as that is apparent.

Agreed; unnecessarily verbose.
I'll remove it.

@capn-freako

capn-freako commented Jun 19, 2026

Copy link
Copy Markdown
Owner Author
  • I assume fix ports and applying a window or eventual manual port reordering should be per file not applied to all files. Which would ripple into the user configuration object.

I think I disagree here.
Here's my reasoning:

  • Fix ports: This merely gives PyBERT the green light to assess the port ordering convention used, changing it only when it finds that the "1->3/2->4" convention was used.
    So, some sections (those having used the "1->3/2->4" convention) would be corrected, while others (those using the "1->2/3->4" convention) would not.

  • Use window: When this option is enabled by the user, the windowing is only applied to the final composite network, not each sub-section individually.

Let's discuss this one some more.

@capn-freako

capn-freako commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner Author
  • Let claude add some test coverage of a single file, multiple files and etc.. Use scikit-rf to generate said networks; make sure the fix ports is applying to all/some or a mix.

Great suggestion!
(And an interesting test of Claude's abilities. ;-) )
I'll poke at that, today.

Okay, checkout 9d629e4 and see if you think the additional testing is sufficient.

capn-freako and others added 4 commits June 19, 2026 07:54
@capn-freako
Removed "Section" from interconnect building buttons, as per David Pa…
dedef32 
…tterson's suggestion.
@capn-freako
Add tests for multi-element composite channel building.
b7effd0 
Uses scikit-rf to construct reference transmission-line networks and verifies
that cascading N s2p files via import_channel matches direct skrf cascades,
including 2- and 3-segment unit tests and an end-to-end PyBERT simulation test.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Prompt used:
> Test the new composite channel building functionality. Use scikit-rf to build the reference channel for testing.
@capn-freako @claude
Expand composite channel tests to cover both s4p port numbering conve…
7bfaf15 
…ntions.

Adds TestPortRenumber with 6 tests verifying that renumber=True correctly
normalises both the standard "1→2" and alternative "1→3" 4-port conventions,
including negative tests (renumber=False on a 1→3 file gives Sdd21≈0) and
mixed-convention cascade scenarios.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Prompt used:
> Expand testing to ensure that port reordering (i.e. - renumber = True) works with a mix of both s4p port numbering conventions.
@capn-freako
Updated CLAUDE.md.
9d629e4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jdpatt jdpatt Awaiting requested review from jdpatt

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add support for multi-element channel modeling.

2 participants

@capn-freako @jdpatt