opensim-org / opensim-core

SimTK OpenSim C++ libraries and command-line applications, and Java/Python wrapping.
https://opensim.stanford.edu
Apache License 2.0
779 stars 311 forks source link

Catch SimTK Exception thrown by Xml::writeToFile() and report likely cause/resolution to user #2160

Open tkuchida opened 6 years ago

tkuchida commented 6 years ago

From Forum topic 9014

OpenSimCreateTugOfWarModel.m (and potentially other MATLAB scripts) can throw an Exception when attempting to print the Model to file. Might be helpful to catch the Exception (either in OpenSim or MATLAB) and suggest a likely cause/resolution (i.e., that the script is being run from a directory to which MATLAB doesn't have permission to write).

https://github.com/opensim-org/opensim-core/blob/e20c2ffb898cd829014344f7739c0a590ba75f16/Bindings/Java/Matlab/examples/OpenSimCreateTugOfWarModel.m#L154

chrisdembia commented 6 years ago

Would it be sufficient to make the Xml::writeToFile() error message in Simbody more informative?

tkuchida commented 6 years ago

Would it be sufficient to make the Xml::writeToFile() error message in Simbody more informative?

Sure (makes no difference to the user), but I thought it had been decided that OpenSim 4.0 would use the Simbody 3.6 release, so a change in Simbody wouldn't trickle down to the user until OpenSim 4.1. The most likely issue in this context is that the user is running a MATLAB script from C:\Program Files; not sure that's the most likely failure mode for Xml::writeToFile() in general.

chrisdembia commented 6 years ago

I think we can make new releases of Simbody for OpenSim 4.0. I didn't know you were suggesting this for inclusion in 4.0.

I think writing to a file has standard error cases (directory doesn't exist, no write permissions, disk is out of space).

tkuchida commented 6 years ago

I didn't know you were suggesting this for inclusion in 4.0.

Not my decision, but I think there are higher priorities---particularly if this fix would require a new Simbody release.

I think writing to a file has standard error cases

Yes, at the Simbody level we would provide the standard error cases. The Exception already says "Failed to write to the Xml file" but that wasn't enough information. A message like "Ensure you are running the .m file from a directory for which MATLAB has write permissions" might be maximally helpful---but it would probably go in the .m file (or, at least, on the OpenSim side) rather than in Simbody.

clnsmith commented 6 years ago

"Ensure you are running the .m file from a directory for which MATLAB has write permissions" -Agreed on this. I'm working with some new users at the moment and they give up when they see the Xml::writeToFile() error. Even though it is something they should be able to debug. I think another common case is writing to a folder that doesn't exist (this is from memory, may not be true).

tkuchida commented 6 years ago

I think another common case is writing to a folder that doesn't exist (this is from memory, may not be true).

Thanks, @clnsmith. You might be thinking of this one: #2179.

clnsmith commented 6 years ago

@tkuchida Yes, that's exactly was I was thinking about.

aseth1 commented 6 years ago

Thanks @tkuchida, @clnsmith and @chrisdembia for raising and discussing the issue. If the Simbody error is not helpful, then we should trap the the SimTK exception in Object::print() and state the two most common reasons for the failure: 1) not a valid path and 2) insufficient permissions to write to the destination folder.