Skip to content

fix: prevent undefined port variable errors in measurement scripts (fixes #345)#401

Merged
pradeeban merged 2 commits into
ControlCore-Project:devfrom
GaneshPatil7517:fix/measurement-undefined-ports
Feb 20, 2026
Merged

fix: prevent undefined port variable errors in measurement scripts (fixes #345)#401
pradeeban merged 2 commits into
ControlCore-Project:devfrom
GaneshPatil7517:fix/measurement-undefined-ports

Conversation

@GaneshPatil7517
Copy link
Copy Markdown
Contributor

@GaneshPatil7517 GaneshPatil7517 commented Feb 18, 2026

Hello pradeeban Sir,

Fixes #345.

Some measurement benchmark scripts (A.py, B.py, C.py) were referencing port variables such as PORT_NAME_F1_F2, PORT_F1_F2, PORT_NAME_F2_F3, and PORT_F2_F3. These variables are normally inserted by copy_with_port_portname.py when a study is generated. However, if the scripts are run directly, those variables are not defined and the scripts crash with NameError.

This PR adds simple fallback definitions so the scripts can still run in standalone mode.

Changes made:

  • measurements/A.py - added fallback for PORT_NAME_F1_F2 and PORT_F1_F2
  • measurements/B.py - added fallback for all four port variables
  • measurements/C.py - added fallback for PORT_NAME_F2_F3 and PORT_F2_F3
  • measurements/readme.md – added a note explaining that these variables are normally injected during study generation

Each script now checks if the variables already exist in globals(). If not, it assigns safe defaults such as "F1_F2", "5555", "F2_F3", and "5556". A warning message is also printed so users know the script is running in standalone mode.

The fallback block is placed after the imports. When copy_with_port_portname.py generates a study, it inserts the real variable definitions before this block, so the injected values take priority and the fallback is skipped.

Result:

  • Scripts no longer crash with NameError when run directly
  • Injected values from study generation still override the fallback
  • Existing workflow remains unchanged
  • A warning message helps guide users when running scripts outside the normal study setup

Scope:

  • Modified only measurement scripts (measurements/A.py, B.py, C.py)
  • Updated measurements/readme.md
  • No changes to concore-lite
  • No changes to Verilog files
  • No changes to the injection mechanism (copy_with_port_portname.py)

Testing done locally:

  • Standalone execution runs without NameError and prints a warning
  • Injected variables override fallback values correctly
  • No warning appears when variables are already defined

Copilot AI review requested due to automatic review settings February 18, 2026 09:18
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes issue #345 by adding fallback definitions for port variables in measurement benchmark scripts (A.py, B.py, C.py) that were previously expected to be injected by copy_with_port_portname.py during study generation. The solution prevents NameError when scripts are executed directly while preserving backward compatibility with the injection mechanism.

Changes:

  • Added globals() checks with safe fallback defaults for undefined port variables in all three measurement scripts
  • Updated readme.md to document the port variable injection dependency and recommended workflow
  • Maintained backward compatibility - injected values take precedence over fallbacks when present

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
measurements/A.py Added fallback definitions for PORT_NAME_F1_F2 and PORT_F1_F2 with defaults "F1_F2" and "5555"
measurements/B.py Added fallback definitions for all four port variables (F1_F2 and F2_F3 pairs) with appropriate defaults
measurements/C.py Added fallback definitions for PORT_NAME_F2_F3 and PORT_F2_F3 with defaults "F2_F3" and "5556"
measurements/readme.md Added section documenting port variable injection dependency and proper execution workflow

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@pradeeban
Copy link
Copy Markdown
Member

Can you check why irrelevant lines are also changed? Is it the EoL character / linebreak? For example, README.md just has some text added to the end. However, the PR shows it as the entire file being replaced. Can you please recreate the PR with minimal line changes? Keeping changes minimal will help with maintaining the code better (for example, on proper use of git blame).

@GaneshPatil7517 GaneshPatil7517 force-pushed the fix/measurement-undefined-ports branch from 3b1d635 to ec5690a Compare February 19, 2026 08:49
@GaneshPatil7517
Copy link
Copy Markdown
Contributor Author

Can you check why irrelevant lines are also changed? Is it the EoL character / linebreak? For example, README.md just has some text added to the end. However, the PR shows it as the entire file being replaced. Can you please recreate the PR with minimal line changes? Keeping changes minimal will help with maintaining the code better (for example, on proper use of git blame).

Hi @pradeeban Sir thank you for the feedback
You were right the previous commit had the entire files showing as changed in the diff. This was caused by the editor rewriting file contents rather than doing a surgical insert, which resulted in unnecessary line-level changes despite the actual code being identical.
I've force-pushed a clean commit (ec5690a) that now contains only the minimal additions:

  • 62 insertions, 1 deletion across 4 files (previously 295 insertions / 233 deletions)
  • All existing lines are untouched no whitespace, EoL, or formatting changes
  • git blame will correctly attribute only the new fallback blocks to this commit
    Please review the updated diff when you get a chance. Thank you...

Comment thread measurements/readme.md Outdated
Reworded the explanation for network communication measurements to enhance clarity and conciseness.
@pradeeban pradeeban merged commit ed13e9e into ControlCore-Project:dev Feb 20, 2026
6 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.

3 participants