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

dcm2niix Fall 2021 Release Candidate #534

Closed
neurolabusc opened this issue Aug 19, 2021 · 26 comments
Closed

dcm2niix Fall 2021 Release Candidate #534

neurolabusc opened this issue Aug 19, 2021 · 26 comments

Comments

@neurolabusc
Copy link
Collaborator

dcm2niix v1.0.20210819 from the development branch is the release candidate for the Fall 2021 stable release. I would appreciate feedback, in particular from teams with large and diverse datasets. Notable changes include:

  • Kludge for incorrect spatial coding of GE ABCD epipolar sequence.
  • Better handling of Philips enhanced DICOMs with huge headers.
  • Consistent conversion of Philips classic DICOMs regardless of instance numbers. Note this will often change the order diffusion volumes are saved to disk, the gradient directions and magnitudes should be changed accordingly to the processed results should remain the same. This may impact processing pipelines, for example eddy_correct defaults to the first volume being the b=0 if reference_no is not set.
  • Consistent (temporal) ordering of Philips enhanced and classic DICOMs.
  • Improved heuristics for determining whether trigger time should be ignored or segmented for various Philips modalities and sequences: issues 395, 428, 499, 506, 533.
  • Improved Siemens MoCo detection to reduce false alarm warnings.

The minimal cmake for the release candidate is:

git clone --branch development https://github.com/rordenlab/dcm2niix.git
cd dcm2niix
mkdir build && cd build
cmake ..
make

The minimal make for the release candidate is:

git clone --branch development https://github.com/rordenlab/dcm2niix.git
cd dcm2niix/console
make
@neurolabusc
Copy link
Collaborator Author

@ghisvail I note that you are listed as the Debian maintainer for dcm2niix. Is there anything I can do to aid this release? I release new versions twice per year (Fall, Spring) and the current stable Debian release bullseye uses an older release (1.0.20201102-1). It would be great if we could keep this current, as the advent of Siemens and Canon enhanced DICOMs have required a lot of changes.

In addition, it would be great to have help with my MRIcron package. The current version on Debian uses the end of life GTK2, and so it would be great to have the QT5 version be the standard. Finally, it would be great to have my MRIcroGL software included with Debian. I am happy to create a .deb package, it would be great to have an expert vet my package and make sure it gets included. Please feel free to email me directly if you have any thoughts.

@ghisvail
Copy link
Contributor

ghisvail commented Sep 9, 2021

Hey Chris,

Is there anything I can do to aid this release?

I know, since I got back to research only recently I have got some catching up to do. I'll find some time to get it updated for the next Ubuntu LTS and Debian cycles.

it would be great to have help with my MRIcron package.

I see the package is still under maintenance by Michael and Yaroslav. If they are no longer active on it I suppose I could step up assuming it's in an acceptable state.

it would be great to have my MRIcroGL software included with Debian.

I agree. The door is open, anyone can contribute it.

I am happy to create a .deb package.

That would be great. Feel free to join Debian Med and file an ITP for it. I'll be happy to review your work and provide pointers, if the debian-med mailing list is not sufficient.

@neurolabusc
Copy link
Collaborator Author

@ghisvail this is great. As an aside, Yaroslav (@yarikoptic) and Michael (@mih) mentioned that they handed over maintenance to the debian-med team (which still includes them, albeit to a much smaller degree). If you agree, I think you would be ideally positioned to take the lead as maintainer for these projects.

  • dcm2niix is a very simple single file command line executable. The only option is whether to compile with the OpenJPEG and CharLS libraries which allow support for some rare transfer syntaxes at the cost of a slightly larger executable and a more modern compiler (CharLS requires C++14).

  • Both MRIcron and MRIcroGL include a lot of additional resource files: color tables, sample images, atlases, scripts (about 70mb in total). My precise understanding for preferred Debian file locations is a bit unclear. Can you confirm and comment on these:

    • /usr/bin/mricrogl would be the executable
    • /usr/share/mricrogl folder contains resources (lut, matcap, atlas, script, shader, python37)
    • /usr/share/applications/mricron.desktop desktop file
    • /usr/share/pixmaps/mricrogl.svg icon for application

@ghisvail
Copy link
Contributor

ghisvail commented Sep 9, 2021

I am not too worried about dcm2niix. It's pretty packaging friendly. We'd have to check whether the manpages are up-to-date though.

Regarding MRIcron and MRIcroGL both are using the Pascal toolchain which I am not familiar with at all. Correct me if I am wrong.

/usr/bin/mricrogl would be the executable

Correct

/usr/share/mricrogl folder contains resources (lut, matcap, atlas, script, shader, python37)

Basically all architecture-independent artifacts (support scripts, atlases, static resources...) goes to /usr/share/microgl. Reviewers might ask you to provide two binary packages out of the mricrogl source package: one microgl package containing the arch-dep executable and one microgl-data containing the arch-indep files, with microgl depending on microgl-data at install time if applicable.

Not sure what python37 corresponds to in this case?

@ghisvail
Copy link
Contributor

ghisvail commented Sep 9, 2021

/usr/share/applications/mricron.desktop desktop file

Correct

/usr/share/pixmaps/mricrogl.svg icon for application

This is the old path. Nowadays, it's under /usr/share/icons/hicolor/scalable/apps/.

Also, you might want to generate a appstream metadata file for easier discovery in software stores.

@neurolabusc
Copy link
Collaborator Author

@ghisvail with regards to the python37 folder, these are the standard libraries for the version of Python embedded into the executable. These are all python scripts, so I believe they are architecture independent (you can run the same script on ARM or Intel CPUs), though they are not system independent (in my experience, they are incompatible between Windows and Unix).

MRIcroGL uses Python for scripting. To ensure compatibility with the architecture and the version, the python interpreter is embedded into the mricrogl executable. However, the embedded executable still needs standard libraries, and these are stored in python37. This is actually a change from prior versions of my software, which would leverage whatever Python and standard libraries existed on the system. The changes from Python 2.7 to 3.* and desire to support old distributions have made this untenable.

I agree the Pascal toolchain is a bit unusual. But it does allow a single code base to natively target GTK2, GTK3, QT5, macOS Cocoa and Windows API, and multiple architectures (at the moment, ARM, x86-64 though is suspect x86-32 and PowerPC/Carbon still work). For Linux and macOS I prefer the LLVM compiler to FPC for performance reasons.

@yarikoptic
Copy link
Contributor

@ghisvail this is great. As an aside, Yaroslav (@yarikoptic) and Michael (@mih) mentioned that they handed over maintenance to the debian-med team (which still includes them, albeit to a much smaller degree). If you agree, I think you would be ideally positioned to take the lead as maintainer for these projects.

FWIW: we are also part of the debian-med team. It was not "handed over", but rather due to our delay with upload to Debian proper, packaging was redone (independently and differently), so we ended up with 2 packaging(s)., and we never aligned. I was given a "go" by original maintainer within debian-med team to just proceed with taking over (aligning) with packaging in debian-med, but I have never got there due to quite different approach. For NeuroDebian, I include all the submodules with test data, making source package quite large, and also not really "uniform" with how packaging is done/maintained within debian-med.

FWIW neurodebian packaging resides in debian branch of http://github.com/neurodebian/dcm2niix . Debian-med https://anonscm.debian.org/git/debian-med/dcm2niix.git and I see that it was updated/uploaded 1.0.20210317-1 recently.

@ghisvail
Copy link
Contributor

ghisvail commented Sep 9, 2021

so I believe they are architecture independent

Then they could go in the data package.

MRIcroGL uses Python for scripting. To ensure compatibility with the architecture and the version, the python interpreter is embedded into the mricrogl executable.

Is that similar to how FSL works? Anyway, embedding the interpreter is gonna be a tough sell for the Debian reviewers. I understand the rationales for it, though python 3.7 will be removed from the archive at some point and then...?

Could it work with the system packaged interpreter instead of the embedded one?

I agree the Pascal toolchain is a bit unusual.

Nothing against it, if it works and the toolchain is still maintained and available then that's all that matters.

I prefer the LLVM compiler to FPC for performance reasons.

I believe that's fine.

@ghisvail
Copy link
Contributor

ghisvail commented Sep 9, 2021

Thanks @yarikoptic for clarifying. NeuroDebian does not operate under the same constraints as Debian-Med does, i.e. we must obey rigorously the Debian packaging standards, whilst ND has got more freedom to package things how they want. I am not surprised some things diverged, and it's absolutely fine.

@neurolabusc
Copy link
Collaborator Author

@ghisvail at compile time, you can choose to have mricrogl use the system installed Python. In my experience, a lot of supercomputer clusters use very old Linux distributions that still use Python 2.7, using the embedded Python helps ensure all MRIcroGL users have a consistent (and relatively modern) Python interpreter.

@ghisvail and @yarikoptic I appreciate your collective wisdom. The spark that got me thinking about this was this users issues. I note that current Ubuntu LTS (bullseye-based 20.04) still uses dcm2niix from 2018 and mricron from 2014. Since Ubuntu is so popular, is there any way to nudge that distribution to update their packages?

See these packages:
https://packages.ubuntu.com/focal/science/mricron
https://packages.ubuntu.com/focal/science/dcm2niix

Versus the Debian releases:
https://packages.debian.org/bullseye/mricron
https://packages.debian.org/source/bullseye/misc/dcm2niix

@yarikoptic
Copy link
Contributor

I note that current Ubuntu LTS (bullseye-based 20.04) still uses dcm2niix from 2018 and mricron from 2014. Since Ubuntu is so popular, is there any way to nudge that distribution to update their packages?

not really AFAIK. As with Debian, a released version of the distribution is "frozen" in terms of accepting new upstream versions. After all it is stable or LTS for a reason -- to provide a stable system, and regardless of upstream's opinion on either a new version is stable -- it was not as well tested on that system etc. As a result, only dedicated (usually fixing security of very important functioning issues) patches could be added to the packages in those released Debian/Ubuntu versions. That is why we created NeuroDebian - to provide alternative (newer, but possibly less tested/more buggy) backport builds on top of those released bases (Debian/Ubuntu).

@ghisvail
Copy link
Contributor

ghisvail commented Sep 9, 2021

using the embedded Python helps ensure all MRIcroGL users have a consistent (and relatively modern) Python interpreter.

That's what Anaconda or Pyenv is for. The system Python interpreter you get is here to support existing packages requiring the Python runtime. If you ship a package for MRIcroGL version X with Python version Y for Debian 12, you are guaranteed both will work together in tandem for the length of the support window.

is there any way to nudge that distribution to update their packages?

Server distros only provide security updates, period.

Users who need more frequent updates either use a different distro (Debian testing, Ubuntu non-LTS, Arch, Fedora, whatever), compile their own (and therefore are responsible for keeping up), or use a different distribution channel (anaconda, nixpkg, ...).

There is a flavor for everyone.

@ghisvail
Copy link
Contributor

ghisvail commented Sep 9, 2021

That is why we created NeuroDebian - to provide alternative (newer, but possibly less tested/more buggy) backport builds on top of those released bases (Debian/Ubuntu).

My bad I forgot to include NeuroDebian in the alternative distribution channels. @yarikoptic summarized well where the value proposition for ND stands.

@ghisvail
Copy link
Contributor

ghisvail commented Sep 9, 2021

not really AFAIK

Technically you can via the backports Debian channel, but 1) it's not enabled by default, 2) Academic IT rarely allows it, 3) It's additional burden for an already limited maintenance workforce. I can count on it for things like nvidia drivers, kernel and mesa updates, for the rest I believe there are better distribution channels.

@neurolabusc
Copy link
Collaborator Author

@ghisvail I am happy to make a first pass at creating Debian packages. It sounds like the recommended way to provide upgrades to Ubuntu LTS is via snaps. Do you have any experience or thoughts about these?

  • Snaps. This is the supported solution for third-party application
    packages on top of Ubuntu. This option would enable you as an upstream to
    directly manage packages for your software and upload it to the Snap
    Store, without any Ubuntu Developer intermediaries required. It would
    require a committment on your part (or on the part of someone else
    involved in the upstream) to be responsible for the packaging and the
    updates to users. The packaging format is fairly straightforward, with
    limited packaging policy to trip you up:
    https://ubuntu.com/tutorials/create-your-first-snap#1-overview

@ghisvail
Copy link
Contributor

Do you have any experience or thoughts about these?

I don't. Snap is technology led by Canonical for Canonical, so there is little incentive for a Debian maintainer to invest time in it. If all you care is Ubuntu, it should be fine I guess.

For third-party desktop application packaging, I personally favor Flatpak, with which I have built a reasonable experience now.

In my opinion, pick one (or each) of those, see how far you go on your own with just the docs and assess whether you'd be happy to maintain it long term. That's what I did personally and what made me chose which technology I preferred between snap and Flatpak.

@yarikoptic
Copy link
Contributor

wouldn't it be better just to stay with NeuroDebian builds? I think I am quite rapidly up to date there.

@neurolabusc
Copy link
Collaborator Author

@yarikoptic I will take you up on that offer! Sticking with .deb files seems easier. I was simply quoting an Ubuntu developer on their advocacy for snaps. However, it sounds like this would only help users with a single distribution, which does not sound like a great long term solution. NeuroDebian has proven to be a terrific resource for our community. I was just exploring other options where I have no personal experience.

@ghisvail
Copy link
Contributor

NeuroDebian has proven to be a terrific resource for our community.

💯

@captainnova
Copy link
Collaborator

Hi, I tested the candidate with our test suite and it passed with flying colors. The test suite was not updated, so it is a test that nothing broke in the bread-and-butter section, not of the new features.

@neurolabusc
Copy link
Collaborator Author

@ghisvail I would like to release the promote the current development version (v1.0.20211006) to be the Fall 2021 stable release. I am unsure if there are any changes you want to suggest for Debian compliance. Once I make a pull request from the development branch to the main branch, the scripts will automatically generate downloadable files for macOS, Windows and Linux. However, the Linux compilation is a simple zip archive of the executable, rather than a proper Debian package that installs the executable to the desired location and generates a manual. Happy to keep either include or exclude Debian specific changes to this repository. Just tell me your preference.

@ghisvail
Copy link
Contributor

ghisvail commented Oct 7, 2021

I would like to release the promote the current development version (v1.0.20211006)

🎉

I am unsure if there are any changes you want to suggest for Debian compliance.

So far so good.

However, the Linux compilation is a simple zip archive of the executable.

Debian uses source releases to produce packages, so we are not concerned by this archive.

As long as a corresponding tag is pushed, everything should be fine on the Debian side.

I am ready for your update 👍

@neurolabusc
Copy link
Collaborator Author

New stable version released v1.0.20211006.

@ghisvail
Copy link
Contributor

@neurolabusc
Copy link
Collaborator Author

Terrific!

@yarikoptic
Copy link
Contributor

thank you @ghisvail ! and I uploaded Neurodebian packaging/build to neurodebian: http://neuro.debian.net/pkgs/dcm2niix.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants