Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/lightning/pytorch/profilers/advanced.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ def start(self, action_name: str) -> None:
def stop(self, action_name: str) -> None:
pr = self.profiled_actions.get(action_name)
if pr is None:
raise ValueError(f"Attempting to stop recording an action ({action_name}) which was never started.")
log.debug(f"Attempting to stop recording an action ({action_name}) which was never started.")
return
pr.disable()

def _dump_stats(self, action_name: str, profile: cProfile.Profile) -> None:
Expand Down
171 changes: 171 additions & 0 deletions test_existing_functionality.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
#!/usr/bin/env python3
"""Test to verify existing AdvancedProfiler functionality still works after the fix.

Based on existing tests from the test suite.

"""

import os

# Import our modified AdvancedProfiler
import sys
import tempfile
import time

sys.path.insert(0, os.path.join(os.path.dirname(__file__), "src"))

from lightning.pytorch.profilers.advanced import AdvancedProfiler


def test_advanced_profiler_deepcopy():
"""Test that AdvancedProfiler can be deep copied."""
from copy import deepcopy

with tempfile.TemporaryDirectory() as tmp_path:
profiler = AdvancedProfiler(dirpath=tmp_path, filename="profiler")
profiler.describe()
try:
deepcopy(profiler)
print("✓ AdvancedProfiler deepcopy works")
return True
except Exception as e:
print(f"✗ AdvancedProfiler deepcopy failed: {e}")
return False


def test_advanced_profiler_nested():
"""Test that AdvancedProfiler handles nested profiling actions."""
with tempfile.TemporaryDirectory() as tmp_path:
profiler = AdvancedProfiler(dirpath=tmp_path, filename="profiler")

try:
with profiler.profile("outer"), profiler.profile("inner"):
pass # Should not raise ValueError
print("✓ AdvancedProfiler nested profiling works")
return True
except Exception as e:
print(f"✗ AdvancedProfiler nested profiling failed: {e}")
return False


def test_advanced_profiler_basic_functionality():
"""Test basic profiling functionality."""
with tempfile.TemporaryDirectory() as tmp_path:
profiler = AdvancedProfiler(dirpath=tmp_path, filename="profiler")

try:
# Test basic profiling
with profiler.profile("test_action"):
time.sleep(0.01) # Small delay to register some activity

# Test that we can get a summary
summary = profiler.summary()
if "test_action" not in summary:
print("✗ test_action not found in profiler summary")
return False

print("✓ AdvancedProfiler basic functionality works")
return True
except Exception as e:
print(f"✗ AdvancedProfiler basic functionality failed: {e}")
return False


def test_advanced_profiler_stop_started_action():
"""Test that stopping a properly started action still works."""
with tempfile.TemporaryDirectory() as tmp_path:
profiler = AdvancedProfiler(dirpath=tmp_path, filename="profiler")

try:
# Start an action
profiler.start("test_action")

# Do some work
time.sleep(0.01)

# Stop the action - this should work
profiler.stop("test_action")

# Verify it's in the summary
summary = profiler.summary()
if "test_action" not in summary:
print("✗ Properly started/stopped action not found in summary")
return False

print("✓ AdvancedProfiler start/stop of real action works")
return True
except Exception as e:
print(f"✗ AdvancedProfiler start/stop of real action failed: {e}")
return False


def test_original_bug_scenario():
"""Test the original bug scenario is now fixed."""
with tempfile.TemporaryDirectory() as tmp_path:
profiler = AdvancedProfiler(dirpath=tmp_path, filename="profiler")

try:
# Simulate the problematic scenario: stop an action that was never started
# This specifically mimics the "run_test_evaluation" error from the issue
profiler.stop("run_test_evaluation")
profiler.stop("run_validation_evaluation")
profiler.stop("some_random_action")

# Verify profiler is still functional after these calls
with profiler.profile("after_fix_test"):
time.sleep(0.01)

summary = profiler.summary()
if "after_fix_test" not in summary:
print("✗ Profiler not functional after stopping nonexistent actions")
return False

print("✓ Original bug scenario is fixed")
return True
except ValueError as e:
print(f"✗ Original bug still exists - ValueError raised: {e}")
return False
except Exception as e:
print(f"✗ Unexpected error in bug fix test: {e}")
return False


def main():
"""Run all tests."""
print("Testing AdvancedProfiler - verifying fix and existing functionality")
print("=" * 70)

tests = [
("Deepcopy functionality", test_advanced_profiler_deepcopy),
("Nested profiling", test_advanced_profiler_nested),
("Basic functionality", test_advanced_profiler_basic_functionality),
("Start/stop real actions", test_advanced_profiler_stop_started_action),
("Original bug fix", test_original_bug_scenario),
]

results = []
for test_name, test_func in tests:
print(f"\n--- {test_name} ---")
results.append(test_func())

print("\n" + "=" * 70)
print("FINAL RESULTS:")

passed = sum(results)
total = len(results)

for i, (test_name, _) in enumerate(tests):
status = "✓ PASS" if results[i] else "✗ FAIL"
print(f"{status}: {test_name}")

print(f"\nSUMMARY: {passed}/{total} tests passed")

if passed == total:
print("✓ ALL TESTS PASSED - Fix is working correctly!")
return 0
print("✗ SOME TESTS FAILED")
return 1


if __name__ == "__main__":
exit(main())
129 changes: 129 additions & 0 deletions test_fix.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
#!/usr/bin/env python3
"""Simple test script to verify the AdvancedProfiler fix works.

This reproduces the original bug and verifies it's fixed.

"""

import os
import sys
import tempfile

# Add the source directory to Python path
sys.path.insert(0, os.path.join(os.path.dirname(__file__), "src"))

try:
from lightning.pytorch.profilers.advanced import AdvancedProfiler

print("✓ Successfully imported AdvancedProfiler")
except ImportError as e:
print(f"✗ Failed to import AdvancedProfiler: {e}")
sys.exit(1)


def test_stop_nonexistent_action():
"""Test that stopping a non-existent action doesn't raise ValueError."""
print("\n=== Testing stop of nonexistent action ===")

with tempfile.TemporaryDirectory() as tmp_dir:
profiler = AdvancedProfiler(dirpath=tmp_dir, filename="test")

try:
# This should NOT raise ValueError after the fix
profiler.stop("run_test_evaluation")
profiler.stop("some_nonexistent_action")
print("✓ Stopping nonexistent actions completed without error")
return True
except ValueError as e:
print(f"✗ ValueError was still raised: {e}")
return False
except Exception as e:
print(f"✗ Unexpected error: {e}")
return False


def test_normal_profiling_still_works():
"""Test that normal profiling functionality still works."""
print("\n=== Testing normal profiling still works ===")

with tempfile.TemporaryDirectory() as tmp_dir:
profiler = AdvancedProfiler(dirpath=tmp_dir, filename="test")

try:
# Normal profiling should still work
with profiler.profile("test_action"):
sum(range(100)) # Some work

# Should be able to get summary
summary = profiler.summary()
if "test_action" in summary:
print("✓ Normal profiling works correctly")
return True
print("✗ Normal profiling summary doesn't contain expected action")
return False

except Exception as e:
print(f"✗ Normal profiling failed: {e}")
return False


def test_mixed_usage():
"""Test mixed usage of stop on nonexistent and normal profiling."""
print("\n=== Testing mixed usage ===")

with tempfile.TemporaryDirectory() as tmp_dir:
profiler = AdvancedProfiler(dirpath=tmp_dir, filename="test")

try:
# Stop nonexistent action - should not error
profiler.stop("nonexistent1")

# Normal profiling
with profiler.profile("real_action"):
sum(range(50))

# Stop another nonexistent action - should not error
profiler.stop("nonexistent2")

# Check summary contains the real action
summary = profiler.summary()
if "real_action" in summary:
print("✓ Mixed usage works correctly")
return True
print("✗ Mixed usage failed - real action not in summary")
return False

except Exception as e:
print(f"✗ Mixed usage failed: {e}")
return False


def main():
"""Run all tests."""
print("Testing AdvancedProfiler fix for issue #9136")
print("=" * 50)

tests = [
test_stop_nonexistent_action,
test_normal_profiling_still_works,
test_mixed_usage,
]

results = []
for test in tests:
results.append(test())

print("\n" + "=" * 50)
print("SUMMARY:")

all_passed = all(results)
if all_passed:
print("✓ ALL TESTS PASSED")
print("✓ The fix successfully resolves the issue!")
return 0
print("✗ SOME TESTS FAILED")
return 1


if __name__ == "__main__":
sys.exit(main())
Loading