Closes #1285_supergzip_rust_implementation_update#1286
Closes #1285_supergzip_rust_implementation_update#1286MauricePasternak wants to merge 5 commits intodevelopfrom
Conversation
HenkMutsaerts
left a comment
HenkMutsaerts
left a comment
There was a problem hiding this comment.
What I don't understand: the original SuperGZip was implemented by Michael for Windows specifically. But now you state that you created something that performs faster (un)zipping in Unix? That would be great of course, but we need to keep xASL_adm_GzipAllFiles to work for all platforms.
Minor name comments:
- a) CamelCase would give
SuperGZip.exe - b) Why do you call it
unzip, thegzipthat you replace does both zip and unzip right?
Should we improve xASL_adm_GzipNifti and xASL_adm_UnzipNifti accordingly?
|
Couple of points:
|
|
@MauricePasternak Thanks for clarifying this! If we make this change, it should work with both multi-core processors and single-threaded processors (if ExploreASL is ran in a server, it could be ran single-threaded but with multiple ExploreASL instances). Can we also address #1052 in here (implementing this also for Linux and macOS)? |
HenkMutsaerts
left a comment
HenkMutsaerts
left a comment
There was a problem hiding this comment.
Nice work! See also my other comment above
| glob_pat = join(['"' ROOT '/**/*.nii"']); | ||
| % Get SuperGzip path | ||
| PathToSuperGzip = fullfile(pathExternal, 'SuperGZip', 'SuperGZip_Windows.exe'); | ||
| PathToSuperGzip = fullfile(pathExternal, 'SuperGZip', 'super-gunzip.exe'); |
There was a problem hiding this comment.
I would still not call it super-gunzip.exe if we also (or even more frequently) use it for zipping; fine to rename to super-zip.exe?
There was a problem hiding this comment.
Sounds good to me. I'll keep calling it super-gunzip in the original repo & its release page. But when copying over to ExploreASL, will change name to just super-zip.
There was a problem hiding this comment.
I would still say that that is weird. No zip package calls itself unzip as primary name right? 7zip not 7unzip. Winrar not WinUnrar. Etc. But perhaps you have another reason to call it this way.
HenkMutsaerts
changed the title
3 participants
Linked issue
Closes #1285
Comments
Tested on 1000+ files in a Population folder and then re-running the study. Noticeable improvement over the old Python version.
As in the original issue, the new version of the program can beat out native gzip in Unix under the condition that at least 3 threads are used. No logic was added for Unix, as this was meant to be a simple language & CLI change, but it wouldn't be difficult to make a check for whether is it reasonable to do so. I leave it to the discretion of reviewers on preferred logic.