opensim-core
opensim-core copied to clipboard
No warning/error if an unexpected XML tag is used
From the forum, a user noted that changing the output_precision
tag in the XML file does not change the precision of the output file in ID: https://simtk.org/plugins/phpBB/viewtopicPhpbb.php?f=91&t=14074&p=0&start=0&view=&sid=9e745b6d243f57bfa1ba0ac8bdad1106
Checking the code, this looks to likely also be true for other tools as well. Testing with the IK files from gait10dof18musc, I also couldn't get output_precision
to change the output file precision.
Some thoughts chatting with @aymanhab:
- 8 decimal places should be enough for most anyone, unless there's some really small model out there that I can't think of?
- Could remove the property
- Or could try to make property work (in case there are use cases out there that need the extra precision, or could reduce file size for some users if they want fewer decimal places)
@aymanhab Update on this that could change some decisions. output_precision
is a property in AbstractTool
, from which AnalyzeTool
, CMCTool
, ForwardTool
, and RRATool
derive. Testing with RRATool
, I could change the property and see a difference in number of digits in the output.
output_precision
is NOT a property in Tool
, from which InverseDynamicsTool
, IMUInverseKinematicsTool
, and InverseKinematicsTool
derive, so this matches what was seen by the user that this doesn't work for ID, and also for my testing in IK.
Questions/Comments:
- Any reason that both
AbstractTool
andTool
exist and don't derive from one another? - We should at least make the behavior the same. At this point, I'm leaning towards making sure that
output_precision
is included inTool
Thanks for the investigation @carmichaelong. Answers
- History: AbstractTool came first and was used as base class for all Tools, then IK, ID were completely rewritten in 2017 based on Tool/DynamicsTool with the intention to replace AbstractTool but that never materialized! The newer Tools use the new Property system while older Tools don't.
- Going with the hypothesis that output_precision is unnecessary, I'd avoid reintroducing it anywhere, instead I'd remove it and see if issues arise. I can explain more of the history if needed.
After discussion with @aymanhab, this is a wider problem that there is no feedback for a user when an incorrect XML tag is given for an object. Would be very helpful for users to give them a warning/error when an XML tag is unused (due to error) so that it's easier to debug.
:+1: Related: #686