Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed aberrant mtr values #2503

Merged
merged 11 commits into from Oct 24, 2019
Merged

Fixed aberrant mtr values #2503

merged 11 commits into from Oct 24, 2019

Conversation

jcohenadad
Copy link
Member

@jcohenadad jcohenadad commented Oct 23, 2019

A combination of issues yielded aberrant MTR values as the output of sct_compute_mtr:

  • The output dtype was based on the input mt1 image. If it was int, it created ugly precision.
  • Division by very small numbers yielded aberrant values (highly dependent on output dtype).

This PR fixes it by:

  • Forcing float32 as the output of mtr
  • Create a "smart" divider, where zeros are removed before division and output values can be clipped given application-specific priors (e.g., MTR should not be higher than 100). The advantage of aggressive clipping is that the output NIFTI file has a "human-friendly" range (e.g. -100/+100), so people don't have to adjust the dynamics when opening the image in a viewer, as can happen sometimes.
  • Added CLI param to change default threshold: -thr
  • SCT version now appears when running the function.

Fixes #2501

@jcohenadad jcohenadad added this to the 4.1.0 milestone Oct 23, 2019
@jcohenadad jcohenadad added bug category: fixes an error in the code sct_compute_mtr context: labels Oct 23, 2019
@@ -13,16 +13,41 @@
logger = logging.getLogger(__name__)


def compute_mtr(nii_mt1, nii_mt0):
def divide_after_removing_zero(dividend, divisor, threshold, replacement=np.nan):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GReat!

type=float,
help="Threshold to clip MTR output values in case of division by small number. This threshold is applied on the"
"absolute value. Default: 100.",
default=100
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So values will be between -100 and 100, is it right? or am I misunderstanding?
If so, could it be helpful to mention this in the help?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed-- clarified in 509c99b

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great!

@jcohenadad jcohenadad merged commit ccfd787 into master Oct 24, 2019
jcohenadad added a commit that referenced this pull request Dec 18, 2019
* sct_compute_mtr: Force saving as float64

* sct_compute_mtr: Changed dtype output to float32 (enough)

* image: Fixed bug where dtype was not set in the nibabel hdr

* qmri/mt: Created function divide_after_removing_zero()

* qmri/mt: Fixed dtype bug

* test_qmri: Now testing with input array

Otherwise new function in qmri/mt is failing

* image: Fixed bug when using SCT-specific dtype 'minimize'

* sct_compute_mtr: Added param -thr

* sct_compute_mtr: now showing logger and SCT version info on run

* qmri/mt: Adding verbose about clipping, nan replacement

* Clarified threshold range in help


Former-commit-id: 7b2e5a195cb355a5080d8eff4327a5c22846c770 [formerly 6c4643d [formerly ccfd787]]
Former-commit-id: b4636e6ad64fd615afc09c4fe796bb84ad1f0593
Former-commit-id: 0b5efe5
jcohenadad added a commit that referenced this pull request Dec 18, 2019
* sct_compute_mtr: Force saving as float64

* sct_compute_mtr: Changed dtype output to float32 (enough)

* image: Fixed bug where dtype was not set in the nibabel hdr

* qmri/mt: Created function divide_after_removing_zero()

* qmri/mt: Fixed dtype bug

* test_qmri: Now testing with input array

Otherwise new function in qmri/mt is failing

* image: Fixed bug when using SCT-specific dtype 'minimize'

* sct_compute_mtr: Added param -thr

* sct_compute_mtr: now showing logger and SCT version info on run

* qmri/mt: Adding verbose about clipping, nan replacement

* Clarified threshold range in help


Former-commit-id: ccfd787
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug category: fixes an error in the code sct_compute_mtr context:
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Aberrant mtr calculation caused by RuntimeWarning: invalid value encountered in true_divide
2 participants