[feature] ONNX Export by Louis-Pagnier-KW · Pull Request #1490 · Kitware/dive
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to push back on any requests I made in my comments. It's been a while since I looked at the desktop code so I may have misinterpreted things.
Within the code the only thing I have a question on is the working directory for the desktop exporting and if we should create one for exporting models. I understand that the model is being directly exported to the save location that user selects. A working directory and the log file would help in debugging any errors during export. I.E I had installed the onnx pipeline and went to export and got a generic error of 'Unable to export model' and the real problem was found in the terminal log being that my pipeline has no trained weights. An actual job log in the job history would make it so I could debug it.
This other request/suggestion is a usability concern with model deletion on the web version. Currently the trained pipeline models you have access to are any that are shared with you. So not just the ones that are in your local trained folder but if a user shared their pipelines you could see and execute them as well. This works for running pipelines but if you were also given write access to the pipeline folder you could delete another user's pipelines. That by itself is fine because before that would require a user to go into someone else folder and delete a file. The new higher level trained models tab is great but it doesn't indicate who the model is from. As admins on viame.kitware.com Matt and I actually would see everyone's pipelines by default because we have access to it. I want to make sure we don't accidently delete someones pipelines as well as users who have pipelines shared (+write access) with them don't accidently delete a pipeline too. In server/dive_server/crud_rpc.py there is a function _load_dynamic_pipelines we may want to include in the pipeline object an optional userId or userName derived from the userId that indicates who a pipeline is from and then in the new Models Tab have that as an additional column and possibly make the data table sortable by user.