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

Added 'save-metrics' option #5341

Closed
wants to merge 9 commits into from

Conversation

SpongeBab
Copy link
Contributor

@SpongeBab SpongeBab commented Oct 26, 2021

Added 'save-metrics' option(default is false):

  • update 'val.py'
  • save the metrics to *.txt

πŸ› οΈ PR Summary

Made with ❀️ by Ultralytics Actions

🌟 Summary

Enhanced logging and performance metrics in YOLOv5 validation script.

πŸ“Š Key Changes

  • πŸ“ Added logging support to capture output in a log file.
  • πŸ’Ύ Introduced the --save-metrics flag to save performance metrics to a text file.

🎯 Purpose & Impact

  • πŸ“ˆ Logging Enhancement: Helps in better error tracking and debugging by recording events and errors in a dedicated log file.
  • πŸ“Š Performance Metrics Saving: Allows users to save detailed validation metrics (precision, recall, mAP) for later analysis or reporting, improving the ability to track model performance over time.

These updates could provide a smoother experience for developers working on model validation and make it easier to monitor and analyze YOLOv5's performance.

* Added 'save-metrics' option
# save the metrics to *.txt
@SpongeBab
Copy link
Contributor Author

@glenn-jocher
I did a lot of experiments before, but when I wanted to get my previous test results, I found that I couldn't find the tested results. I must test all my weights again. So I added this feature.
I tried save-txt, but I found that it saves labels. I think it is necessary to save metrics.

@glenn-jocher
Copy link
Member

@SpongeBab thanks for the PR! This is actually a great idea, but I think we'd want the output to be a csv, similar to results.csv, i.e. a val.csv so that the users know what the columns mean.

I don't think we need an argument for this, we can just generate this automatically if plots is true.

@glenn-jocher
Copy link
Member

@SpongeBab an example csv is here, first row is column names:

if self.csv:
file = self.save_dir / 'results.csv'
n = len(x) + 1 # number of cols
s = '' if file.exists() else (('%20s,' * n % tuple(['epoch'] + self.keys)).rstrip(',') + '\n') # add header
with open(file, 'a') as f:
f.write(s + ('%20.5g,' * n % tuple([epoch] + vals)).rstrip(',') + '\n')

@SpongeBab
Copy link
Contributor Author

@glenn-jocher yeah, I also think we should generate this automatically if plots is true.
So I just need to add:

    # Plots
    if plots:
        confusion_matrix.plot(save_dir=save_dir, names=list(names.values()))
        callbacks.run('on_val_end')

        # save the metrics
        with open(save_dir / 'metrics.csv', 'a') as f:
            f.write(s + '\n'
                    + pf % ('all', seen, nt.sum(), mp, mr, map50, map) + '\n'
                    + f'Speed: %.1fms pre-process, %.1fms inference, %.1fms NMS per image at shape {shape}' % t)

How about this?

And then I found that you use the callback function callbacks.run('on_val_end').
If I want to add these code to callbacks.run('on_val_end'),I need to defined or import these parameters:
pf % ('all', seen, nt.sum(), mp, mr, map50, map),maybe need your help.
I can do these,but I think what I wrote is really not as elegant as your codeπŸ˜‚.

@glenn-jocher
Copy link
Member

glenn-jocher commented Oct 28, 2021

@SpongeBab ah! Ok I finally understand now what we need. CSV has comma separations by the way, but now I realized we don't want CSV, we just want to log the console output to a file. I found a link that can help do this:
https://stackoverflow.com/questions/6386698/how-to-write-to-a-file-using-the-logging-python-module

Though this will also require updates to the loggers pending in #4854

The idea is that anything that gets logged to console (everything you see on your screen) during python val.py will also be saved to runs/val/exp/results.log

# create logger with 'spam_application'
logger = logging.getLogger('spam_application')
logger.setLevel(logging.DEBUG)
# create file handler which logs even debug messages
fh = logging.FileHandler('spam.log')
fh.setLevel(logging.DEBUG)
logger.addHandler(fh)

EDIT: This is really exciting, we can do this for all our main files that create directories in run/, i.e. train.py, val.py, detect.py. This will allow all users to see the exact console output that created those files/results.

@SpongeBab SpongeBab marked this pull request as draft October 29, 2021 03:14
@SpongeBab
Copy link
Contributor Author

yeah,that's right!
Do you have started this work?I think this is a big feature,I'm not similar to the logger,and as you said, this will also require updates to the loggers pending.
Maybe I need long time.Or you can do it. This should be a global feature. It needs a very safe implementation.

@SpongeBab SpongeBab marked this pull request as ready for review October 29, 2021 03:21
@glenn-jocher
Copy link
Member

@SpongeBab yes I need to update #4854 and merge that first. I'll try to get that done soon, and then we can update this PR to create log files by default for all the main functions.

@glenn-jocher
Copy link
Member

@SpongeBab Loggers update is merged now in PR #4854. Every file has a LOGGER global variable that is used to log now, i.e.:

print('message')  # old
LOGGER.info('message')  # new

This should pave the way to use the stackoverflow method in #5341 (comment) to add text logging to log.txt to all the main functions: train.py, detect.py, val.py, export.py

@SpongeBab
Copy link
Contributor Author

SpongeBab commented Nov 3, 2021

@glenn-jocher , I tried https://stackoverflow.com/questions/6386698/how-to-write-to-a-file-using-the-logging-python-module.
I added this line in val.py before save_one_txt:
image
And it generated log.txt In the root directory.

So then I put it under plot:
image
Because I want to save the log.txt to save_dir.But I cann't get any information.
image

And then I put these code in here:
image
Then I successfully save the LOGGER.info() to the save_dir/ log.txt:
image
As you see , there are some garbled. I used utf-8 encoding but did not solve this problem.
Another question is, should I write this function as a function, such as def save_log(). Or a more elegant form?

EDIT: Then I can do the same work on the train,py and detect.py ie.

@glenn-jocher
Copy link
Member

@SpongeBab thanks, I see the updates! Not sure what to do about the color string coding, will search for solutions.

@SpongeBab
Copy link
Contributor Author

SpongeBab commented Nov 4, 2021

@glenn-jocher Hi,I have a question.
How to use the logger = logging.getLogger(), when I use logging.getLogger('yolov5') or logging.getLogger('val') or logging.getLogger('val.py').the log.txt is empty.
As I show in the screeshot , I set logger = logging.getLogger() and can get the log.
Another thing, the LOGGER.info of print_args(FILE.stem, opt) cann't save to the log.txt.
Maybe it is because parse_opt() is before main(opt)
image

Take it your way~

@glenn-jocher
Copy link
Member

@SpongeBab LOGGER filehandler correctly handles print_args(), but it does not handle tqdm sources like the progress bar for example. I've added updates to val.py as an example of how this might work. We can't run this in save_dir since it's not defined yet, so we probably want to save to ROOT / log.txt and then move log.txt to save_dir if it exists on function end.

Screen Shot 2021-11-04 at 4 14 38 PM

@SpongeBab
Copy link
Contributor Author

SpongeBab commented Nov 5, 2021

@glenn-jocher Hh, One line just.Yeah, I did this in the beginning, while I need to use save_dir.
Then I meet a problem.I tried move log.txt to save_dir on function end.
image

This is the output:
image

The process cannot access the file beacause it is being used by another process.
Then I search for solution:
https://github.com/Preston-Landers/concurrent-log-handler.

Report to you.

@SpongeBab
Copy link
Contributor Author

And report a small bug.
image
increment_path cann't handle decimals.

@glenn-jocher
Copy link
Member

glenn-jocher commented Nov 5, 2021

@SpongeBab this is interesting. By complete coincidence I just fixed increment_paths() to handle decimal places in #5518 to fix another use case. Can you merge master and try again to see if #5518 fixes your issue too?

EDIT: the increment_paths() fix allows it to make new directories containing decimals now. This is the only change. If you increment a directory with decimals it will increment it as a file:

path/to/dir.abc.def
path/to/dir.abc2.def  # increment decimal directory
path/to/dir.abc3.def  # increment decimal directory (I should probably fix so number is at end)

@SpongeBab
Copy link
Contributor Author

TODO:

  • Search for the solution for the color string coding.
  • concurrent-log-handler?

glenn-jocher added a commit that referenced this pull request Nov 5, 2021
Resolves #5341 (comment)

Tests:
```python
import glob
import re
from pathlib import Path


def increment_path(path, exist_ok=False, sep='', mkdir=False):
    # Increment file or directory path, i.e. runs/exp --> runs/exp{sep}2, runs/exp{sep}3, ... etc.
    path = Path(path)  # os-agnostic
    if path.exists() and not exist_ok:
        path, suffix = (path.with_suffix(''), path.suffix) if path.is_file() else (path, '')
        dirs = glob.glob(f"{path}{sep}*")  # similar paths
        matches = [re.search(rf"%s{sep}(\d+)" % path.stem, d) for d in dirs]
        i = [int(m.groups()[0]) for m in matches if m]  # indices
        n = max(i) + 1 if i else 2  # increment number
        path = Path(f"{path}{sep}{n}{suffix}")  # increment path
    if mkdir:
        path.mkdir(parents=True, exist_ok=True)  # make directory
    return path


print(increment_path('runs'))
print(increment_path('export.py'))
print(increment_path('abc.def.dir'))
print(increment_path('abc.def.file'))
```
@SpongeBab
Copy link
Contributor Author

@glenn-jocher Oh, you closed it, Sorry for that, I can't solve the two problems.

@glenn-jocher
Copy link
Member

@SpongeBab ah sorry, didn't mean to close.

@glenn-jocher
Copy link
Member

@SpongeBab sorry, this keeps getting closed for some reason. Did you find any good methods to log to text within save_dir? I think we can ignore the color errors. It's possible they might actually be read in color by the appropriate editor.

@glenn-jocher
Copy link
Member

/rebase

@SpongeBab
Copy link
Contributor Author

SpongeBab commented Nov 11, 2021

@glenn-jocher yes,I have looked for some ways to record color string. I tried to search for plug-ins that can identify color string in pycharm. I did find it. But this kind of plugins only support:

For color option, the value like #xxx/#xxxxxx/#xxxxxxxx can be recognized, the already defined will be reused without regenerating a new one.

I have not found a way to save the color string codes you are using.
Colors a string https://en.wikipedia.org/wiki/ANSI_escape_code
Unless we no longer use this method or use a color string code that a plugin can recognize.


As for [WinError 32], unless I save it in the root path. Otherwise, will encounter this error.

@github-actions
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions YOLOv5 πŸš€ and Vision AI ⭐.

@github-actions github-actions bot added the Stale label Dec 12, 2021
@github-actions github-actions bot closed this Dec 18, 2021
BjarneKuehl pushed a commit to fhkiel-mlaip/yolov5 that referenced this pull request Aug 26, 2022
Resolves ultralytics#5341 (comment)

Tests:
```python
import glob
import re
from pathlib import Path


def increment_path(path, exist_ok=False, sep='', mkdir=False):
    # Increment file or directory path, i.e. runs/exp --> runs/exp{sep}2, runs/exp{sep}3, ... etc.
    path = Path(path)  # os-agnostic
    if path.exists() and not exist_ok:
        path, suffix = (path.with_suffix(''), path.suffix) if path.is_file() else (path, '')
        dirs = glob.glob(f"{path}{sep}*")  # similar paths
        matches = [re.search(rf"%s{sep}(\d+)" % path.stem, d) for d in dirs]
        i = [int(m.groups()[0]) for m in matches if m]  # indices
        n = max(i) + 1 if i else 2  # increment number
        path = Path(f"{path}{sep}{n}{suffix}")  # increment path
    if mkdir:
        path.mkdir(parents=True, exist_ok=True)  # make directory
    return path


print(increment_path('runs'))
print(increment_path('export.py'))
print(increment_path('abc.def.dir'))
print(increment_path('abc.def.file'))
```
SecretStar112 added a commit to SecretStar112/yolov5 that referenced this pull request May 24, 2023
Resolves ultralytics/yolov5#5341 (comment)

Tests:
```python
import glob
import re
from pathlib import Path


def increment_path(path, exist_ok=False, sep='', mkdir=False):
    # Increment file or directory path, i.e. runs/exp --> runs/exp{sep}2, runs/exp{sep}3, ... etc.
    path = Path(path)  # os-agnostic
    if path.exists() and not exist_ok:
        path, suffix = (path.with_suffix(''), path.suffix) if path.is_file() else (path, '')
        dirs = glob.glob(f"{path}{sep}*")  # similar paths
        matches = [re.search(rf"%s{sep}(\d+)" % path.stem, d) for d in dirs]
        i = [int(m.groups()[0]) for m in matches if m]  # indices
        n = max(i) + 1 if i else 2  # increment number
        path = Path(f"{path}{sep}{n}{suffix}")  # increment path
    if mkdir:
        path.mkdir(parents=True, exist_ok=True)  # make directory
    return path


print(increment_path('runs'))
print(increment_path('export.py'))
print(increment_path('abc.def.dir'))
print(increment_path('abc.def.file'))
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants