Implements MTsat function #1672
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zougloub
reviewed
Apr 10, 2018
spinalcordtoolbox/mtsat/mtsat.py
Outdated
| nii_b1map=None, verbose=1): | ||
| """ | ||
| Compute MTsat and T1 map based on FLASH scans | ||
| Parameters |
zougloub
reviewed
Apr 10, 2018
spinalcordtoolbox/mtsat/mtsat.py
Outdated
| tr_t1 *= 0.001 | ||
|
|
||
| # Convert flip angles into radians | ||
| fa_mt_rad = math.radians(fa_mt) |
There was a problem hiding this comment.
When the rest of the code overwhelmingly uses numpy, you might as well use np.radians() here.
zougloub
reviewed
Apr 10, 2018
spinalcordtoolbox/mtsat/mtsat.py
Outdated
|
|
||
| # ignore warnings from division by zeros (will deal with that later) | ||
| # note: do not use over='ignore' because it results in wrong T1 maps | ||
| seterr_old = np.seterr(over='ignore', divide='ignore', invalid='ignore') |
There was a problem hiding this comment.
Comment says « do not use over='ignore' because it results in wrong T1 maps » but code does it. Is the comment up-to-date?
There was a problem hiding this comment.
ah shit ;-) not up to date
zougloub
reviewed
Apr 10, 2018
|
|
||
| def run_main(args): | ||
| import sct_utils as sct | ||
| from spinalcordtoolbox.mtsat import mtsat |
There was a problem hiding this comment.
Could mtsat be in a folder that may contain other "similar" tools?
from x.something import something to import a module is not the prettiest.
There was a problem hiding this comment.
Agreed, however I believe this requires group discussion. Opened an issue here
zougloub
requested changes
Apr 12, 2018
spinalcordtoolbox/mtsat/mtsat.py
Outdated
|
|
||
| # compute MTsat | ||
| nii_mtsat, nii_t1map = compute_mtsat(nii_mt, nii_pd, nii_t1, tr_mt, tr_pd, tr_t1, fa_mt, fa_pd, fa_t1, | ||
| nii_b1map=None, verbose=verbose) |
There was a problem hiding this comment.
The parameter nii_b1map is always None despite having loaded nii_b1map before?
There was a problem hiding this comment.
thanks, fixed in a3f9d23
zougloub
approved these changes
May 4, 2018
jcohenadad
added a commit
that referenced
this pull request
May 7, 2018
* master: Implements MTsat function (#1672) Keep intermediate temp folder by default (#1694) sct_deepseg_sc - Issue when input is .nii instead of .nii.gz (#1634, #1659) sct_image: remove file removal kludge (#1688) (#1705) Concatenate string to keep previously assigned subtitle field (fixes #1690) (#1692) Enable input file with label at a specific disc (#1698) # Conflicts: # spinalcordtoolbox/gui/sagittal.py
jcohenadad
added a commit
that referenced
this pull request
May 9, 2018
* master: (31 commits) Control the brightness of the image in the GUI. (#1684) Refactor things dealing with dataset `info_label.txt` metadata file (#1676) Implements MTsat function (#1672) Keep intermediate temp folder by default (#1694) sct_deepseg_sc - Issue when input is .nii instead of .nii.gz (#1634, #1659) sct_image: remove file removal kludge (#1688) (#1705) Concatenate string to keep previously assigned subtitle field (fixes #1690) (#1692) Enable input file with label at a specific disc (#1698) Use python concurrent.futures instead of multiprocessing (#1587) Fixup benign installation failures from #1679 (#1680) output of -display ordered per label value (#1686) Slice counting fixed (#1687) Travis: binaries: install libvtk6 or libvtk7 Travis: isct_propseg: install more deps Travis: attempt to build propseg isct_propseg: link with ITKIOTransformBase isct_propseg: some C++ clean-up Rearranged the imports and removed the cyclic imports. (#1511) msct_register_landmarks: fixup default path_qc sct_warp_template: WarpTemplate: remove dead ctor argument ...
jcohenadad
added a commit
that referenced
this pull request
Dec 18, 2019
* sct_compute_mtsat: added functions * fixed argument issues * fixed wrong initialization location * manage output files * cosmetic change * clip unrealistic T1 and MTsat values (unfinished) * fixed issue introduced by discarding overflow warnings * now ignoring overflow only for computing MTsat * added viewer at the end * sct_utils:display_viewer_syntax: added hsv option * adjusted output viewer colormap * make sure np.seterr are set back to original before quitting function * now imposing elementwise multiplication in float to prevent overflow * fixed another multiplication with float * removed old comment * switched to reST comment style * removed math dependency * BUG: fixed wrong passing of argument Former-commit-id: ade1abe
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requirements
Description of the Change
Implementation of the MTsat and T1 mapping function based on Helms et al. This implementation is also a test-bench for various SCT coding guidelines that have recently been discussed. Notably:
argparseinstead ofmsct_parseras suggested in Replace usage of custom command-line parser (msct_parser) with argparse #1548Applicable Issues
Implements #1073