fix negative scale factor of coordinate function due to spatial transform axis direction #3992
Conversation
…avoid -ve scale factors depending on translation axis specification
|
Hi Dr. Habib, Using absolute values from transform axis when computing aggregate scale factor is the most appropriate fix to the issue in my opinion. Thank you very much for taking swift action! Geng Li |
| double scaleFactor = ~axis * scaleFactors; | ||
| // we want weighted aggregate of scale factors but to ignore the sign | ||
| // ignoring sign due to issue #3991 resulting -ve scale factor | ||
| double scaleFactor = ~axis.abs() * scaleFactors; |
There was a problem hiding this comment.
Seems like a reasonable enough change; but it makes me wonder: is there any scenario where we would want to preserve the sign on the scale factor?
There was a problem hiding this comment.
While manual scales can be set to -ve numbers, measurement-based scale factors are ratios between (unsigned) distances so can't be -ve. If we think about it as how much we stretch/shrink segments then this should never be -ve otherwise vectors/directions flip which is unlikely an intended behavior of our 'scaling'. However if you suspect there're other places in the code worth more investigation please let me know @nickbianco
There was a problem hiding this comment.
Sounds good, just making sure I understood this correctly.
|
Great, thanks @nickbianco 👍 |
Fixes issue #3991
Brief summary of changes
Use absolute values from transform axis when computing aggregate scale factor to avoid erroneous sign flip
Testing I've completed
Tested model/files from bug report on simtk.org
Looking for feedback on...
CHANGELOG.md (choose one)
This change is