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

Detect target precision profile #167

Draft
wants to merge 23 commits into
base: main
Choose a base branch
from
Draft

Conversation

KarthiU
Copy link

@KarthiU KarthiU commented Feb 26, 2024

Created profiler_detect_target_2024.py and modified detect_target.py to add timing functionality.

profiler_detect_target_2024.py

  • added dependencies on timing, pandas, and numpy libraries
  • Structured on around the main_2024.py file, but removed flight interface related code
  • Created a profiler.txt file that holds raw data used for analysis
  • Added indicator to config.yaml to stop the main loop after x seconds have passed (define in the config)
  • Gets logged data and interprets it, outputting the min, max, average, median and initial times for preprocessing, inference, postprocessing and total elapsed time of the detect_target prediction loop

detect_target.py

  • If full precision is not overriden to true, it defaults to false (for testing purposes only)
  • saves timing data to profiler.txt

config.yaml

  • added profiling_time to set how long you profile for (i.e. 300 = 5 min)

Copy link
Contributor

@mgupta27 mgupta27 left a comment

Choose a reason for hiding this comment

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

Added some comments.

Comment on lines +85 to +89
elapsed_time = stop_time - start_time

for pred in predictions:
with open('profiler.txt', 'a') as file:
speeds = pred.speed
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't want to add logic within the detect_target.py class for profiling. Rather we could time the worker outside of the call to detect target.

Copy link
Collaborator

@Xierumeng Xierumeng Feb 26, 2024

Choose a reason for hiding this comment

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

The worker isn't the thing to test, something like this:

# profiling_or_whatever_file_name.py

def profile_detector(detector: detect_target.DetectTarget, images: "list[np.ndarray]") -> ...:
    for image in images:
        gc.disable()  # This disables the garbage collector
        start = time.time_ns()
        result, value = detector.run(image)  # Might or might not want to keep the bounding boxes
        end = time.time_ns()
        gc.enable()  # This enables the garbage collector
        if not result:
            # Handle error
      
        # Save results somewhere
        time_ns = end - start
        ...

def main() -> int:
    images = load_many_images()
    detector_half = detect_target.DetectTarget(...)
    detector_full = detect_target.DetectTarget(...)

    # Initial run just to warm up CUDA
    _ = profile_detector(detector_full, images[:10])
    time_half = profile_detector(detector_half, images)
    time_full = profile_detector(detector_full, images)

    # Record the results
    ...

Comment on lines 30 to 31
self.__enable_half_precision = False if self.__device == "cpu" else False
#modified so override_full controls if its half or full - FOR PROFILING ONLY
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of modifying the logic here, when profiling why not just specify the device type and the current logic will handle if half or full precision should be used?

Copy link
Collaborator

@Xierumeng Xierumeng Feb 26, 2024

Choose a reason for hiding this comment

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

As another PR, it might be worth updating this interface to a create() method that takes a settings object as argument. The settings class could have a is_setting_valid() method, which can push the responsibility of ensuring a valid setting to the caller. This also increases transparency because there won't be any fallthrough logic (the setting is applied as is).

Comment on lines 25 to 29
def main() -> int:
"""
copied from airside code main function
"""
# Open config file
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think you could explain what the difference between this file is and profile_detect_target.py? Just trying to understand what the difference is.

@Xierumeng
Copy link
Collaborator

It would also be better to use a dataset that are not all identical images. Perhaps the same as used in repository profile-encode.

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.

5 participants