ultralytics icon indicating copy to clipboard operation
ultralytics copied to clipboard

Custom folder for export artifacts

Open mayorblock opened this issue 1 year ago โ€ข 5 comments

In some situations it can be useful to be able to specify the output location of exported artifacts (for example, training on remove and storing on some mounted storage). This is an answer to #4503 that discuss this option in the context of ONNX. It turned out that with minimal changes it can work for most (all?) the rest which are included

I somewhat arbitrarily picked artifact_path for the new optional configuration parameter (I am very much open for better alternatives :).

I have had to split the artifact location from the input model location; they where tightly coupled before. It works with both absolute and relative (to current working directory). Currently it copies the artifact name from the input model but that could of course be adjusted, if you find that useful.

I hope you find this contribution useful and I very much looks forward to hear your opinions / comments.

I have extended and run the unit tests as much as possible but work on a mac. I was not able to run some of the LINUX-enabled only formats. Also I have not yet looked into enabling this in the cli (but will do)

Cheers Mikael

๐Ÿ› ๏ธ PR Summary

Made with โค๏ธ by Ultralytics Actions

๐ŸŒŸ Summary

Improved model export functionality with temporary directories for safer file handling.

๐Ÿ“Š Key Changes

  • ๐Ÿ›  Added tempfile usage to create temporary directories during model exports.
  • ๐Ÿ”„ Changed model export tests to use these temporary directories to ensure that files are handled in a safe and controlled environment.
  • ๐ŸŽจ Minor code reorganization and optimizations across various files for better readability and performance.

๐ŸŽฏ Purpose & Impact

  • ๐ŸŽ‰ Enhanced Safety: Using temporary directories during model exports avoids conflicts and potential data loss, making the process safer and more reliable.
  • ๐Ÿงน Code Cleanup: The reorganization and minor optimizations help maintain the codebase, making it easier to manage and understand.
  • ๐Ÿš€ User Experience: These changes contribute to a more robust and error-resistant environment, potentially improving user satisfaction by minimizing unexpected issues.

mayorblock avatar May 09 '24 20:05 mayorblock

All Contributors have signed the CLA. โœ…
Posted by the CLA Assistant Lite bot.

github-actions[bot] avatar May 09 '24 20:05 github-actions[bot]

Codecov Report

Attention: Patch coverage is 85.71429% with 3 lines in your changes missing coverage. Please review.

Project coverage is 68.45%. Comparing base (b3dfbdd) to head (f64d141). Report is 418 commits behind head on main.

Files with missing lines Patch % Lines
ultralytics/engine/exporter.py 84.21% 3 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11817      +/-   ##
==========================================
- Coverage   70.63%   68.45%   -2.18%     
==========================================
  Files         122      122              
  Lines       15631    15635       +4     
==========================================
- Hits        11041    10703     -338     
- Misses       4590     4932     +342     
Flag Coverage ฮ”
Benchmarks 35.37% <85.71%> (-0.17%) :arrow_down:
GPU ?
Tests 66.66% <57.14%> (-0.09%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar May 10 '24 19:05 codecov[bot]

I have read the CLA Document and I sign the CLA

mayorblock avatar May 12 '24 18:05 mayorblock

@mayorblock have you investigated the idea of using the ckpt_path attribute instead? This would mean that a new argument wouldn't be required. Currently the model.model.pt_path attribute is used, but it would be nice to use an existing attribute or argument to accomplish the same functionality, as we're already contending with a large number of arguments. I did a quick test to see if using the pt_path attribute would change the output filename, and it did update as expected.

from ultralytics import YOLO

model = YOLO("yolov8n.pt")
model.model.pt_path = "yolov8n-custom-name.pt"
out = model.export(format="onnx")

print(out)
>>> 'yolov8n-custom-name.onnx'

This did not work if I added a directory model.model.pt_path = "new_path/yolov8n-custom-name.pt" so if you wanted to try to create a directory using this, code would need to be added to verify the directory existed and to create one if not (this might work if the directory exists). I did not attempt with an absolute directory elsewhere with this example, so you may find that could be problematic or adds complexity.

If possible, it would be nice to use a higher level attribute (like the previously mentioned model.ckpt_path or even model.model_name) to set the output location instead. There's also the project and name arguments that are used with training, which might be another area to investigate, but I suspect the attributes might be a simpler solution.

Burhan-Q avatar May 13 '24 12:05 Burhan-Q

@Burhan-Q thank you for the suggestions. I did notice that model.pt_path was used for export location, also with (existing) folder included. I would definitely prefer not to use this one to specify export location (as I understand it this is not what you are suggestion either).

I had more distinct decoupling in mind but your point to avoid introduction of additional (minor) arguments is a very valid one. I will have a look at model.ckpt_path to see it if can work and get back with a suggestion.

mayorblock avatar May 13 '24 18:05 mayorblock

HI @Burhan-Q , I liked my initial idea so much (and did not really need the new functionality :) so it took a little while to get back into this. A few people have expressed interest so I will try to finish it.

I will bounce a few ideas before making more changes:

  • Of the ideas you mention I prefer ckpt_path and would like to allow model.export(format="..", ckpt_path="/custom_folder/custom_name.format"). However, it seems like I would need to either (i) add it as a key to custom.yaml or change the override logic in the export function. Would any of the two be acceptable? or do you have another suggestion?
  • I don't find project or name intuitive; they remind me more of experiment tracking (especially given associated comments in default.yaml)
  • model.model.pt_path seems to be the easiest (extending it to create any missing folders) but I am afraid the use would be a little obscure.

Looking forward to hear your thoughts :)

mayorblock avatar Sep 14 '24 20:09 mayorblock