Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEAT: Identify coupling 5649 #5659

Merged
merged 35 commits into from
Feb 4, 2025
Merged

FEAT: Identify coupling 5649 #5659

merged 35 commits into from
Feb 4, 2025

Conversation

amichel0205
Copy link
Contributor

@amichel0205 amichel0205 commented Jan 15, 2025

Description

This feature is useful for large RF board where user want to identified unwanted coupling.
User will specify:

Sparameter file full path
Log file full path
low_loss and high_loss range
Frequency start at which to start looking for the coupling(such we can skip low frequency where DC block exist)
Sample frequency(to speed up process chack coupling at each sample)

Issue linked

Close #5649

Checklist

  • I have tested my changes locally.
  • I have added necessary documentation or updated existing documentation.
  • I have followed the coding style guidelines of this project.
  • I have added appropriate tests (unit, integration, system).
  • I have reviewed my changes before submitting this pull request.
  • I have linked the issue or issues that are solved by the PR if any.
  • I have agreed with the Contributor License Agreement (CLA).

@amichel0205 amichel0205 added the enhancement New features or code improvements label Jan 15, 2025
@amichel0205 amichel0205 self-assigned this Jan 15, 2025
@ansys-reviewer-bot
Copy link
Contributor

Thanks for opening a Pull Request. If you want to perform a review write a comment saying:

@ansys-reviewer-bot review

@amichel0205 amichel0205 changed the title Identify coupling 5649 FEAT:Identify coupling 5649 Jan 15, 2025
Copy link

codecov bot commented Jan 15, 2025

Codecov Report

Attention: Patch coverage is 86.11111% with 5 lines in your changes missing coverage. Please review.

Project coverage is 85.21%. Comparing base (8214b24) to head (6410a3d).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5659   +/-   ##
=======================================
  Coverage   85.20%   85.21%           
=======================================
  Files         156      156           
  Lines       61071    61107   +36     
=======================================
+ Hits        52035    52070   +35     
- Misses       9036     9037    +1     

@amichel0205 amichel0205 changed the title FEAT:Identify coupling 5649 FEAT: Identify coupling 5649 Jan 15, 2025
@amichel0205 amichel0205 requested review from Samuelopez-ansys and hui-zhou-a and removed request for hui-zhou-a and SMoraisAnsys January 27, 2025 07:59
Copy link
Contributor

@hui-zhou-a hui-zhou-a left a comment

Choose a reason for hiding this comment

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

@amichel0205 please use a valid s-parameter for testing.

tests/system/general/test_44_TouchstoneParser.py Outdated Show resolved Hide resolved
@Samuelopez-ansys Samuelopez-ansys enabled auto-merge (squash) February 3, 2025 14:59
Copy link
Member

@Samuelopez-ansys Samuelopez-ansys left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@SMoraisAnsys SMoraisAnsys left a comment

Choose a reason for hiding this comment

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

I left some refactoring proposition.

tests/system/general/test_44_TouchstoneParser.py Outdated Show resolved Hide resolved
@amichel0205 amichel0205 requested review from SMoraisAnsys and removed request for hui-zhou-a February 4, 2025 09:38
hui-zhou-a
hui-zhou-a previously approved these changes Feb 4, 2025
Copy link
Contributor

@hui-zhou-a hui-zhou-a left a comment

Choose a reason for hiding this comment

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

LGTM

@Samuelopez-ansys Samuelopez-ansys dismissed SMoraisAnsys’s stale review February 4, 2025 13:10

Requested changes implemented

@Samuelopez-ansys Samuelopez-ansys merged commit cd7ed64 into main Feb 4, 2025
42 checks passed
@Samuelopez-ansys Samuelopez-ansys deleted the identify_coupling_5649 branch February 4, 2025 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New features or code improvements
Projects
None yet
4 participants