Skip to content

Conversation

@aseembits93
Copy link
Contributor

@aseembits93 aseembits93 commented May 28, 2025

PR Type

Enhancement


Description

  • Simplify progress_bar call invocation

  • Move benchmark start message to logger.info


Changes walkthrough 📝

Relevant files
Enhancement
optimizer.py
Simplify progress_bar and add log message                               

codeflash/optimization/optimizer.py

  • Removed message and transient flag in progress_bar
  • Added logger.info for benchmark start
  • +2/-1     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Progress bar configuration

    The new progress_bar() call no longer takes a description or transient flag. Verify that the bar still provides context (e.g., a header or label) and is properly cleaned up when finished.

    with progress_bar():
        logger.info(f"Running benchmarks in {self.args.benchmarks_root}")

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Restore progress bar title

    Pass the benchmark message and transient flag directly to the progress bar instead
    of using a separate logger call, so the title appears alongside the bar and clears
    automatically when done. Remove the redundant logger.info line.

    codeflash/optimization/optimizer.py [105-106]

    -with progress_bar():
    -    logger.info(f"Running benchmarks in {self.args.benchmarks_root}")
    +with progress_bar(f"Running benchmarks in {self.args.benchmarks_root}", transient=True):
    Suggestion importance[1-10]: 8

    __

    Why: The suggestion correctly restores the original progress bar invocation with title and transient=True, improving UX by displaying and clearing the title automatically.

    Medium
    Print status to console

    Use a direct print statement for immediate console feedback, ensuring the user sees
    the progress message even if the logging configuration filters out info-level logs.

    codeflash/optimization/optimizer.py [106]

    -logger.info(f"Running benchmarks in {self.args.benchmarks_root}")
    +print(f"Running benchmarks in {self.args.benchmarks_root}")
    Suggestion importance[1-10]: 2

    __

    Why: Replacing logger.info with print bypasses logging configuration and is a minimal change that may introduce inconsistency and is generally less preferable.

    Low

    if self.args.benchmark and num_optimizable_functions > 0:
    with progress_bar(f"Running benchmarks in {self.args.benchmarks_root}", transient=True):
    with progress_bar(f"Running benchmarks in {self.args.benchmarks_root}",transient=True):
    logger.info(f"Running benchmarks in {self.args.benchmarks_root}")
    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    this looks like a case where the above message prints in interactive console mode but doesn't in non-interactice consoles?

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

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

    I am closing this issue as this can be solved in Pycharm by selecting 'Emulate terminal in output console' in the run configuration

    @aseembits93
    Copy link
    Contributor Author

    ProgressBar might not work reliably in all kinds of terminals

    @aseembits93
    Copy link
    Contributor Author

    I am closing this issue as this can be solved in Pycharm by selecting 'Emulate terminal in output console' in the run configuration

    @aseembits93 aseembits93 deleted the add-console-msg-benchmark branch June 6, 2025 23:55
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    3 participants