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

⚡️ Speed up method HotPotQAEnv.construct_lookup_list by 28% in PR #99 (hotpotqa_lookup_fix2) #101

Closed

Conversation

codeflash-ai[bot]
Copy link

@codeflash-ai codeflash-ai bot commented Oct 28, 2024

⚡️ This pull request contains optimizations for PR #99

If you approve this dependent PR, these changes will be merged into the original PR branch hotpotqa_lookup_fix2.

This PR will be automatically closed if the original PR is merged.


📄 HotPotQAEnv.construct_lookup_list() in packages/hotpotqa/src/aviary/envs/hotpotqa/env.py

📈 Performance improved by 28% (0.28x faster)

⏱️ Runtime went down from 4.10 milliseconds to 3.21 milliseconds (best of 5 runs)

Explanation and details

Here's the optimized version of your Python program. I've focused on improving the logic without changing the function signatures or renaming functions.

Explanation of Optimizations.

  1. Removed if s.strip() and keyword.lower() in s.lower().

    • Perform s.strip() inside the list comprehension only once for constructing the results.
    • Avoid redundant s.strip() checks, as the condition s.strip() guarantees non-empty strings.
  2. Checked for keyword.lower() Once.

    • Store the lowercase version of the keyword (keyword_lower) instead of computing it multiple times in the list comprehension. This reduces redundant calls to keyword.lower(), improving runtime performance, especially with long texts.

Correctness verification

The new optimized code was tested for correctness. The results are listed below.

🔘 (none found) − ⚙️ Existing Unit Tests

✅ 61 Passed − 🌀 Generated Regression Tests

(click to show generated tests)
# imports
import pytest  # used for our unit tests
# function to test
from aviary.env import Environment
from aviary.envs.hotpotqa.env import HotPotQAEnv


# unit tests
class MockState:
    def __init__(self):
        self.done = False
        self.page = None
        self.last_action = None
        self.last_lookup = None
        self.lookup_results = []
        self.lookup_index = 0
        # Outputs were verified to be equal to the original implementation

@pytest.fixture
def env():
    # Create a mock environment with a mock state
    env = HotPotQAEnv(question="What is HotPotQA?", correct_answer="HotPotQA")
    env.state = MockState()
    return env
    # Outputs were verified to be equal to the original implementation

def test_basic_multiple_occurrences(env):
    # Basic test case: keyword present multiple times
    env.state.page = ("Milhouse is a character in The Simpsons.\n"
                      "Milhouse is Bart's best friend.\n"
                      "Milhouse often gets into trouble.\n"
                      "Bart and Milhouse have many adventures.")
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation

def test_basic_single_occurrence(env):
    # Basic test case: keyword present once
    env.state.page = ("Bart is a character in The Simpsons.\n"
                      "Milhouse is Bart's best friend.\n"
                      "Bart often gets into trouble.\n"
                      "Bart and Milhouse have many adventures.")
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation

def test_basic_no_occurrence(env):
    # Basic test case: keyword not present
    env.state.page = ("Bart is a character in The Simpsons.\n"
                      "Bart is Lisa's brother.\n"
                      "Bart often gets into trouble.\n"
                      "Bart and Lisa have many adventures.")
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation

def test_edge_empty_keyword(env):
    # Edge case: empty keyword
    env.state.page = ("Bart is a character in The Simpsons.\n"
                      "Milhouse is Bart's best friend.\n"
                      "Bart often gets into trouble.\n"
                      "Bart and Milhouse have many adventures.")
    codeflash_output = env.construct_lookup_list("")
    # Outputs were verified to be equal to the original implementation

def test_edge_empty_page(env):
    # Edge case: empty page content
    env.state.page = ""
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation

def test_edge_special_characters(env):
    # Edge case: keyword with special characters
    env.state.page = ("Milhouse Mussolini Van Houten is a character in The Simpsons.\n"
                      "Milhouse is Bart's best friend.\n"
                      "Bart and Milhouse have many adventures.")
    codeflash_output = env.construct_lookup_list("Mussolini")
    # Outputs were verified to be equal to the original implementation

def test_edge_mixed_case_keyword(env):
    # Edge case: keyword with mixed case
    env.state.page = ("Milhouse is a character in The Simpsons.\n"
                      "Milhouse is Bart's best friend.\n"
                      "Bart and Milhouse have many adventures.")
    codeflash_output = env.construct_lookup_list("milhouse")
    # Outputs were verified to be equal to the original implementation

def test_state_consecutive_different_keywords(env):
    # State management: consecutive lookups with different keywords
    env.state.page = ("Milhouse is a character in The Simpsons.\n"
                      "Bart is Milhouse's best friend.\n"
                      "Milhouse often gets into trouble.\n"
                      "Bart and Milhouse have many adventures.")
    codeflash_output = env.construct_lookup_list("Milhouse")
    codeflash_output = env.construct_lookup_list("Bart")
    # Outputs were verified to be equal to the original implementation

def test_state_consecutive_same_keyword(env):
    # State management: consecutive lookups with the same keyword
    env.state.page = ("Milhouse is a character in The Simpsons.\n"
                      "Milhouse is Bart's best friend.\n"
                      "Milhouse often gets into trouble.\n"
                      "Bart and Milhouse have many adventures.")
    codeflash_output = env.construct_lookup_list("Milhouse")
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation

def test_state_no_prior_action(env):
    # State management: lookup with no prior action
    env.state.page = ("Milhouse is a character in The Simpsons.\n"
                      "Milhouse is Bart's best friend.\n"
                      "Milhouse often gets into trouble.\n"
                      "Bart and Milhouse have many adventures.")
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation

def test_large_scale_large_page(env):
    # Large scale: large page content
    env.state.page = ("Milhouse is a character in The Simpsons.\n" * 1000)
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation

def test_large_scale_high_frequency_keyword(env):
    # Large scale: high frequency keyword
    env.state.page = ("The quick brown fox jumps over the lazy dog.\n" * 1000)
    codeflash_output = env.construct_lookup_list("the")
    # Outputs were verified to be equal to the original implementation

def test_special_keyword_at_beginning(env):
    # Special scenario: keyword at the beginning of sentences
    env.state.page = ("Milhouse is a character in The Simpsons.\n"
                      "Bart is Milhouse's best friend.\n"
                      "Milhouse often gets into trouble.\n"
                      "Bart and Milhouse have many adventures.")
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation

def test_special_keyword_at_end(env):
    # Special scenario: keyword at the end of sentences
    env.state.page = ("A character in The Simpsons is Milhouse.\n"
                      "Bart's best friend is Milhouse.\n"
                      "Often gets into trouble, Milhouse does.\n"
                      "Many adventures are had by Bart and Milhouse.")
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation

def test_special_keyword_in_middle(env):
    # Special scenario: keyword in the middle of sentences
    env.state.page = ("In The Simpsons, Milhouse is a character.\n"
                      "Bart's best friend, Milhouse, is often in trouble.\n"
                      "Many adventures are had by Bart and Milhouse.")
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation

def test_error_invalid_state_page_not_set(env):
    # Error handling: invalid state (page not set)
    env.state.page = None
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation

def test_error_state_reset_after_completion(env):
    # Error handling: state reset after completion
    env.state.page = ("Milhouse is a character in The Simpsons.\n"
                      "Milhouse is Bart's best friend.\n"
                      "Milhouse often gets into trouble.\n"
                      "Bart and Milhouse have many adventures.")
    env.construct_lookup_list("Milhouse")
    env.construct_lookup_list("Milhouse")
    env.construct_lookup_list("Milhouse")
    codeflash_output = env.construct_lookup_list("Milhouse")
    codeflash_output = env.construct_lookup_list("Bart")
    # Outputs were verified to be equal to the original implementation

def test_performance_large_number_of_sentences(env):
    # Performance: large number of sentences
    env.state.page = ("Milhouse is a character in The Simpsons.\n" * 10000)
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation

def test_performance_large_keyword(env):
    # Performance: large keyword
    env.state.page = ("A very long keyword is here: Milhouse.\n" * 1000)
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation

def test_boundary_keyword_at_split_boundary(env):
    # Boundary condition: keyword at the boundary of sentence splits
    env.state.page = ("Milhouse is a character in The Simpsons.\n"
                      "Bart is Milhouse's best friend.\n"
                      "Milhouse often gets into trouble.\n"
                      "Bart and Milhouse have many adventures.")
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation

def test_boundary_keyword_in_multiline_sentences(env):
    # Boundary condition: keyword in multi-line sentences
    env.state.page = ("Milhouse is a character in The Simpsons.\n"
                      "Bart is Milhouse's best friend.\n"
                      "Milhouse often gets into trouble.\n"
                      "Bart and Milhouse have many adventures.")
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation

def test_mixed_content_keyword_with_punctuation(env):
    # Mixed content: keyword with different punctuation
    env.state.page = ("Milhouse! is a character in The Simpsons.\n"
                      "Bart is Milhouse's best friend.\n"
                      "Milhouse, often gets into trouble.\n"
                      "Bart and Milhouse; have many adventures.")
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation

def test_mixed_content_embedded_keywords(env):
    # Mixed content: sentences with embedded keywords
    env.state.page = ("Milhouse is a character in The Simpsons.\n"
                      "Bart is Milhouse's best friend.\n"
                      "Milhouse often gets into trouble.\n"
                      "Bart and Milhouse have many adventures.")
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation
# imports
import pytest  # used for our unit tests
# function to test
from aviary.env import Environment
from aviary.envs.hotpotqa.env import HotPotQAEnv

# unit tests

class MockState:
    def __init__(self):
        self.page = ""
        self.done = False
        self.last_action = None
        self.last_lookup = None
        self.lookup_results = []
        self.lookup_index = 0
        # Outputs were verified to be equal to the original implementation

@pytest.fixture
def env():
    mock_state = MockState()
    env = HotPotQAEnv(
        question="Who is Milhouse?",
        correct_answer="Milhouse",
        correct_reward=1.0,
        incorrect_reward=0.0,
        tool_failure_reward=0.0,
        proxy=None
    )
    env.state = mock_state
    return env
    # Outputs were verified to be equal to the original implementation

def test_single_keyword_match(env):
    env.state.page = "Milhouse is Bart's best friend. Milhouse loves Lisa."
    codeflash_output = env.construct_lookup_list("Milhouse")
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation

def test_multiple_keyword_matches(env):
    env.state.page = "Bart is a troublemaker. Bart loves skateboarding. Milhouse is Bart's best friend."
    codeflash_output = env.construct_lookup_list("Bart")
    codeflash_output = env.construct_lookup_list("Bart")
    codeflash_output = env.construct_lookup_list("Bart")
    # Outputs were verified to be equal to the original implementation

def test_empty_keyword(env):
    env.state.page = "Milhouse is Bart's best friend."
    codeflash_output = env.construct_lookup_list("")
    # Outputs were verified to be equal to the original implementation

def test_empty_page_content(env):
    env.state.page = ""
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation

def test_keyword_not_found(env):
    env.state.page = "Milhouse is Bart's best friend."
    codeflash_output = env.construct_lookup_list("Homer")
    # Outputs were verified to be equal to the original implementation

def test_keyword_case_insensitivity(env):
    env.state.page = "Milhouse is Bart's best friend. MILHOUSE loves Lisa."
    codeflash_output = env.construct_lookup_list("milhouse")
    codeflash_output = env.construct_lookup_list("milhouse")
    # Outputs were verified to be equal to the original implementation

def test_extra_whitespace_in_sentences(env):
    env.state.page = "  Bart is a troublemaker. \n Bart loves skateboarding.  "
    codeflash_output = env.construct_lookup_list("Bart")
    codeflash_output = env.construct_lookup_list("Bart")
    # Outputs were verified to be equal to the original implementation

def test_empty_lines_in_page_content(env):
    env.state.page = "Bart is a troublemaker.\n\nBart loves skateboarding."
    codeflash_output = env.construct_lookup_list("Bart")
    codeflash_output = env.construct_lookup_list("Bart")
    # Outputs were verified to be equal to the original implementation

def test_multiple_calls_with_same_keyword(env):
    env.state.page = "Bart is a troublemaker. Bart loves skateboarding."
    codeflash_output = env.construct_lookup_list("Bart")
    codeflash_output = env.construct_lookup_list("Bart")
    # Outputs were verified to be equal to the original implementation

def test_multiple_calls_with_different_keywords(env):
    env.state.page = "Bart is a troublemaker. Bart loves skateboarding."
    codeflash_output = env.construct_lookup_list("Bart")
    codeflash_output = env.construct_lookup_list("Bart")
    env.state.page = "Milhouse is Bart's best friend."
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation

def test_large_page_content(env):
    env.state.page = ("Bart is a troublemaker.\n" * 1000) + ("Bart loves skateboarding.\n" * 1000)
    codeflash_output = env.construct_lookup_list("Bart")
    codeflash_output = env.construct_lookup_list("Bart")
    # Outputs were verified to be equal to the original implementation

def test_high_frequency_keyword(env):
    env.state.page = "Milhouse is Bart's best friend. Bart is a troublemaker. Lisa is smart."
    codeflash_output = env.construct_lookup_list("is")
    codeflash_output = env.construct_lookup_list("is")
    codeflash_output = env.construct_lookup_list("is")
    # Outputs were verified to be equal to the original implementation

def test_sentences_with_punctuation(env):
    env.state.page = "Bart is a troublemaker! Bart loves skateboarding, and he is good at it."
    codeflash_output = env.construct_lookup_list("Bart")
    codeflash_output = env.construct_lookup_list("Bart")
    # Outputs were verified to be equal to the original implementation

def test_sentences_with_special_characters(env):
    env.state.page = "Bart is a troublemaker @ Springfield Elementary. Bart loves skateboarding #cool."
    codeflash_output = env.construct_lookup_list("Bart")
    codeflash_output = env.construct_lookup_list("Bart")
    # Outputs were verified to be equal to the original implementation

def test_unicode_characters(env):
    env.state.page = "Milhouse is Bart's best friend. ミルハウスはバートの親友です。"
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation

def test_state_reset_between_calls(env):
    env.state.page = "Bart is a troublemaker. Bart loves skateboarding."
    codeflash_output = env.construct_lookup_list("Bart")
    codeflash_output = env.construct_lookup_list("Bart")
    env.state = MockState()  # Reset state
    env.state.page = "Milhouse is Bart's best friend."
    codeflash_output = env.construct_lookup_list("Milhouse")
    # Outputs were verified to be equal to the original implementation

🔘 (none found) − ⏪ Replay Tests

albertbou92 and others added 6 commits October 27, 2024 14:51
… (`hotpotqa_lookup_fix2`)

Here's the optimized version of your Python program. I've focused on improving the logic without changing the function signatures or renaming functions.



### Explanation of Optimizations.

1. **Removed `if s.strip() and keyword.lower() in s.lower()`**.
   - Perform `s.strip()` inside the list comprehension only once for constructing the results.
   - Avoid redundant `s.strip()` checks, as the condition `s.strip()` guarantees non-empty strings.

2. **Checked for `keyword.lower()` Once**.
   - Store the lowercase version of the keyword (`keyword_lower`) instead of computing it multiple times in the list comprehension. This reduces redundant calls to `keyword.lower()`, improving runtime performance, especially with long texts.
@codeflash-ai codeflash-ai bot added the ⚡️ codeflash Optimization PR opened by Codeflash AI label Oct 28, 2024
@dosubot dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Oct 28, 2024
@dosubot dosubot bot added the enhancement New feature or request label Oct 28, 2024
@codeflash-ai codeflash-ai bot closed this Oct 28, 2024
Base automatically changed from hotpotqa_lookup_fix2 to main October 28, 2024 23:03
Copy link
Author

codeflash-ai bot commented Oct 28, 2024

This PR has been automatically closed because the original PR #99 by albertbou92 was closed.

@codeflash-ai codeflash-ai bot deleted the codeflash/optimize-pr99-2024-10-28T21.18.19 branch October 28, 2024 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚡️ codeflash Optimization PR opened by Codeflash AI enhancement New feature or request size:XL This PR changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant