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

Added creation of bvec and bval files when creating NIfTI files #335

Merged

Conversation

cmadjar
Copy link
Collaborator

@cmadjar cmadjar commented Aug 13, 2018

For DWI files, the binary mnc2nii is not creating the .bvec and .bval files necessary to obtain the direction informations. Added support in the imaging pipeline so that those files can be created automatically within the function make_nii of the MRI.pm file.

Behaviour of the script:

  • if there are values in the MINC header acquisition:bvalues then create the .bval file using these values
  • if there are values in the MINC headers acquisition:direction_x, acquisition:direction_y and acquisition:direction_z then create the .bvec file using these values

Notes for existing projects

A script has been created in the tools directory to create the missing bval/bvec files for projects that have the 'create_nii' configuration set to 'Yes'. This script is called create_nifti_bval_bvec.pl.

screen shot 2018-08-15 at 10 08 55 am

Notes for people that want to create those files from a different script

Simply run mnc2nii, then call the MRI.pm functions create_dwi_nifti_bval_file and create_dwi_nifti_bvec_file to create the .bval and .bvec files. Args to the function are:

  • file hash ref (after loading the file via the File.pm library
  • full path to the .bval or .bvec file

…WI files (when bvalue and direction are found in the MINC header)
@cmadjar
Copy link
Collaborator Author

cmadjar commented Aug 13, 2018

@MounaSafiHarab As I created the PR, I just thought I should probably create a tool script to back populate the .bvec and .bval files for projects that have NIfTI creation turned on. What do you think?

@cmadjar
Copy link
Collaborator Author

cmadjar commented Aug 15, 2018

@MounaSafiHarab @nicolasbrossard this should be ready for review :)

my $sth = $dbh->prepare($query);
$sth->execute('acquisition:bvalues');

my @file_ids;
Copy link
Collaborator

Choose a reason for hiding this comment

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

my @file_ids = map { $_->{'FileID'} }  @{ $sth->fetchall_arrayref( {} ) };

...but if you think it is too cryptic, leave the current code as is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done. Thanks for the suggestion!


# determine paths for bval/bvec files
my $minc = $file->getFileDatum('File');
my ($bval_file, $bvec_file) = map { $minc }(0 .. 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since you are creating an array of identical elements, use the repetiton (x) operator:

my ($bval_file, $bvec_file) = ($minc) x 2;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A lot simpler. Thanks!


# determine paths for bval/bvec files
my $minc = $file->getFileDatum('File');
my ($bval_file, $bvec_file) = map { $minc }(0 .. 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since you are matching the end of the file name, the g pattern match modifier is not needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

right. Removed the g pattern match modifier

$bvec_file =~ s/mnc$/bvec/g;

# create complementary nifti files for DWI acquisitions
my $bval_success = NeuroDB::MRI::create_dwi_nifti_bval_file(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like "$data_dir/$bval_file" better than $data_dir . '/' . $bval_file

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done!

my $bval_success = NeuroDB::MRI::create_dwi_nifti_bval_file(
\$file, $data_dir . '/' . $bval_file
);
my $bvec_success = NeuroDB::MRI::create_dwi_nifti_bvec_file(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like "$data_dir/$bvec_file" better than $data_dir . '/' . $bvec_file

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done


# clean up the bvals string
$bvals =~ s/\.\,//g; # remove all '.,' from the string
$bvals =~ s/\.$//g; # remove the last trailing '.' from the string
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unnecessary g.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

# print bvals into bval_file
write_to_file($bval_file, $bvals, '>');

return 1 if (-e $bval_file);
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could also do

return -e $bval_file;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

did not think of that. Nice!

$mode = '>>'; # set to 0 to append to the file the other rows
}

return 1 if (-e $bvec_file);
Copy link
Collaborator

Choose a reason for hiding this comment

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

return -e $bvec_file;


return undef unless @bvecs;

# loop through all bvecs, clean them up and print them into the bvec file
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think opening the file, writing to it and then closing it on each iteration is not a good idea. I wouldn't even define a function to print to a file

s/^\"+|\"$//g for @bvec;
open(OUT, '>', $bvec_file) or die "Cannot write to file $bvec_file: $!\n";
print OUT map { "$_\n" } @bvec;
close(OUT);

uploadNeuroDB/NeuroDB/MRI.pm Show resolved Hide resolved
$file->getParameter('acquisition:direction_y'),
$file->getParameter('acquisition:direction_z')
);

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think @bvecs will always contain 3 elements and so will always evaluate to true. return undef unless @bvecs is unnecessary IMHO. Have you ever come across such a case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, for T1W acquisitions for example, those 3 elements are not there and it will return undef, skipping the bval/bvec creation since there are no fields to create them. This avoids having to add another config setting to specify the scan type for which to create bval/bvec files (which would be a config setting a bit obscure). So if those headers are present, continue in this function to create the bvec file, if they are not present, then return from the function and don't create bvec file. Does that make more sense?

Copy link
Collaborator Author

@cmadjar cmadjar left a comment

Choose a reason for hiding this comment

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

@nicolasbrossard Thank you for your feedback!! They should all have been taken care of. Please re-review.

my $sth = $dbh->prepare($query);
$sth->execute('acquisition:bvalues');

my @file_ids;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done. Thanks for the suggestion!


# determine paths for bval/bvec files
my $minc = $file->getFileDatum('File');
my ($bval_file, $bvec_file) = map { $minc }(0 .. 1);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A lot simpler. Thanks!


# determine paths for bval/bvec files
my $minc = $file->getFileDatum('File');
my ($bval_file, $bvec_file) = map { $minc }(0 .. 1);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

right. Removed the g pattern match modifier

$bvec_file =~ s/mnc$/bvec/g;

# create complementary nifti files for DWI acquisitions
my $bval_success = NeuroDB::MRI::create_dwi_nifti_bval_file(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done!

my $bval_success = NeuroDB::MRI::create_dwi_nifti_bval_file(
\$file, $data_dir . '/' . $bval_file
);
my $bvec_success = NeuroDB::MRI::create_dwi_nifti_bvec_file(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

my $nifti = $minc;
$nifti =~ s/mnc$/nii/g;
my ($nifti, $bval_file, $bvec_file) = map { $minc }(0 .. 2);
$nifti =~ s/mnc$/nii/g;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done. As well as the ($minc) x 2 instead of map { $minc }(0 .. 2)


# mnc2nii command
my $m2n_cmd = "mnc2nii -nii -quiet " .
$data_dir . "/" . $minc . " " .
$data_dir . "/" . $nifti;
system($m2n_cmd);

# create complementary nifti files for DWI acquisitions
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

indeed. A lot clearer. I also did that for the command four lines before and it all fits in one line :)


# clean up the bvals string
$bvals =~ s/\.\,//g; # remove all '.,' from the string
$bvals =~ s/\.$//g; # remove the last trailing '.' from the string
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

# print bvals into bval_file
write_to_file($bval_file, $bvals, '>');

return 1 if (-e $bval_file);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

did not think of that. Nice!

$file->getParameter('acquisition:direction_y'),
$file->getParameter('acquisition:direction_z')
);

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, for T1W acquisitions for example, those 3 elements are not there and it will return undef, skipping the bval/bvec creation since there are no fields to create them. This avoids having to add another config setting to specify the scan type for which to create bval/bvec files (which would be a config setting a bit obscure). So if those headers are present, continue in this function to create the bvec file, if they are not present, then return from the function and don't create bvec file. Does that make more sense?

$bvals =~ s/\.$//; # remove the last trailing '.' from the string

# print bvals into bval_file
open(FILE, '>', $bval_file) or die "Could not open file $bval_file $!";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing semi-colon in error message (see corresponding message in create_dwi_nifti_bvec_file).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done in the next commit!

@nicolasbrossard nicolasbrossard removed the request for review from MounaSafiHarab September 13, 2018 15:34
@cmadjar
Copy link
Collaborator Author

cmadjar commented Sep 13, 2018

@nicolasbrossard Fixed the missing colon. Thank you! Ready for review again :)

@cmadjar cmadjar merged commit a8972fc into aces:20.1-dev Sep 13, 2018
@cmadjar cmadjar deleted the creation_bvec_bval_after_mnc2nii_conversion branch September 13, 2018 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants