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
remove minc, add a minc based on SeriesUID #138
Conversation
|
|
|
@@ -744,10 +775,10 @@ sub dicom_to_minc { | |||
############################################################ | |||
#### use some other converter if specified in the config ### | |||
############################################################ | |||
if ($converter ne 'dcm2mnc') { | |||
if ($converter !~ /dcm2mnc/) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change allows project to modify their prod files to use a different version of dcm2mnc and still use the same command format. For example dcm2mnc4ng does match the proper command string now.
Wow! Someone is having fun!!! Regarding the pull request. I did not look at it closely but I thought you would like to know. A unique minc is defined by the combination of the SeriesUID field and the EchoTime field. This is particularly important when dealing with acquisitions having multiple echoes (like the field maps, the MP2RAGE for example). Therefore, you might want to modify your pull request to include the EchoTime field as well. Feel free to contact me if you have questions. I'll also be back next week. Cheers!! Cécile |
I'm not sure I follow @cmadjar.. if they have multiple echos in a single acquisition, how can the echo be part of the acquisition identification? |
Here is an example that could help clarifying. A field map acquisition on Siemens scanners typically results in three images:
In the case of the magnitude files, if you only check the SeriesUID, one of the image will never be inserted because the first one with the same SeriesUID was already inserted. This is why you need the EchoTime in addition to the SeriesUID in order to check for file uniqueness. This is also true for other modalities using multi-echos. We have now a new modality (quantitative T2* map) in PREVENT-AD using 12 different echo times but the SeriesUID remains the same for the 12 images. We can talk more during the next meeting. |
use NeuroDB::MRIProcessingUtility; | ||
|
||
my $profile = 'prod'; # this should never be set unless you are in a | ||
# stable production environment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
based on the comment, shouldn't this not be set by default?
my $dbh = &NeuroDB::DBI::connect_to_db(@Settings::db); | ||
|
||
|
||
sub selORdel { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a little confusing to have a function with the same name as a variable, when they're two different things (I'm a little surprised perl allows it at all)
@cmadjar @MounaSafiHarab Can one of you double check that the changes to Other than that, this looks fine to me. |
"name of config file in ../dicom-archive/.loris_mri" | ||
], | ||
["-seriesuid", "string", 1, \$seriesuid, "Only deletes this SeriesUID" | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gluneau
can you please add description of the confirm and check if seriesUID is not set?
|
usage: $0 [options] select|confirm | ||
$0 -help to list options | ||
|
||
USAGE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gluneau
nice help section!
It would be nice to explain what the select and confirm options are supposed to do. It is not very clear.
I used this command using the tarchiveLoader and it reinserted everything. Shouldn't it have inserted only the scan corresponding to that seriesUID? tarchiveLoader -globLocation -profile prod tarchive_file.tar -seriesuid 1.3.12.2.1107.5.2.32.35442.2016071310164138453406379.0.0.0 |
@cmadjar, I found that same bug at the same time! I think at some point I was overzealous and I removed the parenthesis, but the are essential and not optional! line 123 of MRIProcessingUtility.pm my ($seriesuid) = @_; |
let me know when you have taken care of it and I'll test the insertion once more :) |
Great job! Thank you :) |
A thought that just crossed my mind: |
print "\nDelete from DB"; | ||
# Delete from DB | ||
selORdel("parameter_file","Value"); | ||
selORdel("files_qcstatus","QCStatus"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
look at the delete of lines 238 and 239;
before deleting from these two tables; you should have a user input to confirm this is to be deleted
(can map the information and then when re-inserting the new file, can use SeriesUID and EchoTime to re-insert the QC information for the newly inserted FileID)
New script to delete minc file based on SeriesUID
uploadNeuroDB/minc_deletion.pl -profile prod -seriesuid 1.3.12.2.1107.5.2.32.35045.201507232316035928639007.0.0.0 confirm
Also, modification of tarchiveLoader so that a unique file can be inserted based on SeriesUID
uploadNeuroDB/tarchiveLoader -globLocation -profile null_grads -seriesuid 1.3.12.2.1107.5.2.32.35045.201507232316035928639007.0.0.0 DCM_2015-07-23_ImagingUpload-10-38-WiBmLY.tar