search

dcmjs-org / dcmjs

Javascript implementation of DICOM manipulation
https://dcmjs.netlify.com/
MIT License
291 stars 110 forks source link

fix: 🐛 relax condition in nearlyEquals check for detecting numbers near to zero #304

Closed Punzo closed 1 year ago

Punzo commented 1 year ago

Fix https://github.com/OHIF/Viewers/issues/2869, see https://github.com/OHIF/Viewers/issues/2869#issuecomment-1228331103 for more info

Punzo commented 1 year ago

@pieper can you please review and merge, please?

pieper commented 1 year ago

@Punzo thanks for quick action on this 👍

Let's review it together on Thursday to make sure we fully understand what we want the behavior to be here. At the least we should update the comment since we are no longer following the logic of the source function from here: https://floating-point-gui.de/errors/comparison/. The current comment implies the code is still just a transliteration to javascript.

What I want to get a handle on is what a valid value of tolerance should be for data we reasonably expect to be displayed.

Since a common use case seems to be going to and from 32-bit float in a nifti1 header I'd like to take into account what rounding errors we are likely to see and set the tolerance to something that will be reasonable and robust.

Punzo commented 1 year ago

@Punzo thanks for quick action on this +1

Let's review it together on Thursday to make sure we fully understand what we want the behavior to be here. At the least we should update the comment since we are no longer following the logic of the source function from here: https://floating-point-gui.de/errors/comparison/. The current comment implies the code is still just a transliteration to javascript.

What I want to get a handle on is what a valid value of tolerance should be for data we reasonably expect to be displayed.

Since a common use case seems to be going to and from 32-bit float in a nifti1 header I'd like to take into account what rounding errors we are likely to see and set the tolerance to something that will be reasonable and robust.

Sure, let's discuss it next Thursday at the OHIF-IDC call.

github-actions[bot] commented 1 year ago

:tada: This PR is included in version 0.24.10 :tada:

The release is available on:

Your semantic-release bot :package::rocket: