This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 721112 - (vmtk) Review Request: vmtk - The Vascular Modeling Toolkit
Review Request: vmtk - The Vascular Modeling Toolkit
Status: ASSIGNED
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
Depends On: 483663 InsightToolkit vxl 705885 OpenNL
Blocks: FE-SCITECH fedora-medical
  Show dependency treegraph
 
Reported: 2011-07-13 14:40 EDT by Ankur Sinha (FranciscoD)
Modified: 2015-08-20 10:33 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
hobbes1069: fedora‑review?


Attachments (Terms of Use)

  None (edit)
Description Ankur Sinha (FranciscoD) 2011-07-13 14:40:42 EDT
Spec URL: http://ankursinha.fedorapeople.org/vmtk/vmtk.spec
SRPM URL: http://ankursinha.fedorapeople.org/vmtk/vmtk-0.9.0-1.fc15.src.rpm

Description: 
== Segmentation of vascular segments (or other anatomical structures) from medical
images:

    Gradient-based 3D level sets segmentation. A new gradient computation
modality based on upwind finite differences allows the segmentation of small
(down to 1.2 pixels/diameter) vessels.
    Interactive level sets initialization based on the Fast Marching Method.
This includes a brand new method for selecting a vascular segment comprised
between two points automatically ignoring side branches, no parameters involved.
Segmenting a complex vascular tract comes down to selecting the endpoints of a
branch, letting level sets by attracted to gradient peaks with the sole
advection term turned on, repeating the operation for all the branches and
merging everything in a single model. 

== Geometric analysis and surface data processing of 3D models of blood vessels
(and tubular objects in general)((The key algorithms have been published on
medical imaging journals. You can find a complete reference to publications at
David Steinman's and Luca Antiga's homepages)):

    Compute centerlines and maximal inscribed sphere radius of branching tubular
structures given their polygonal surface representation
    Split surface models into their constitutive branches based on centerline
geometry
    Compute centerline-based geometric quantities (such as bifurcation angles,
planarity, symmetry, branch curvature, tortuosity) and surface-based geometric
quantities (such as distance to centerlines, surface curvature, deviation from
tangency to maximal inscribed spheres)
    Robustly map branches to a rectangular parametric space
    Generate rectangular patches based on the parametric mapping for statistical
analysis of geometric and CFD data over populations. 

== Scripts, I/O tools and simple algorithms to easily work with images and meshes:

    Read and write a number of image, surface and volume mesh formats. Includes
a DICOM series reader with auto-flipping capabilities, Netgen mesh format
reader, libMesh xda mesh format writer, Tetgen mesh generator wrapper, FIDAP
FDNEUT mesh format reader and writer and a Newtetr input file generator
    Display images and meshes
    Incapsulate several VTK classes and make them available as pipeable scripts
(e.g. Marching Cubes, surface smoothing, clipping, normal computation,
connectivity, subdivision, distance between surfaces, ICP registration)
    Add cylindrical extensions to surface model boundaries as a preprocessing
step for CFD simulations.
    Generate boundary layers of prismatic elements with varying thickness for
CFD 

=========================================================================

Some rpmlint issues. Fixing and uploading a new version in a day or two.
Comment 1 Ankur Sinha (FranciscoD) 2011-07-14 12:50:21 EDT
Cleaned up rpmlint errors. Now there are only warnings

http://ankursinha.fedorapeople.org/vmtk/vmtk-0.9.0-2.fc15.src.rpm

http://ankursinha.fedorapeople.org/vmtk/vmtk.spec
Comment 2 Richard Shaw 2011-07-19 12:01:54 EDT
I'll take this one for you.

Some quick observations.

1. The %description is WAY TO LONG. 

2. I think the -devel package requires should include %{?_isa} since this isn't a noarch pacakge:

Requires:  %{name}%{?_isa} = %{version}-%{release}

3. Could use more comments especially in %prep. I assume you're mainly removing bundled libraries there.

Don't worry about posting a new release just yet until I can do a more in-depth spec review.
Comment 3 Ankur Sinha (FranciscoD) 2011-07-19 13:39:58 EDT
Thank you Richard! :)
Comment 4 Richard Shaw 2011-07-19 14:53:04 EDT
I've made some progress trying to get it to build against the OpenNL package but I can't really test it because of:

rpmbuild -bc rpmbuild/vmtk/SPECS/vmtk.spec
error: Failed build dependencies:
        InsightToolkit-devel is needed by vmtk-0.9.0-2.fc14.x86_64
        vxl-devel is needed by vmtk-0.9.0-2.fc14.x86_64

Because of that I've added the review request of vxl and InsightToolkit as a blocker for this Review Request.

Back to this review :)

The only part of the project that appears to use OpenNL is the DifferentialGeometry sub-project. 

I've added a cmake option, USE_EXTERN_OPENNL to the main CMakeLists.txt and I'm working on getting the headers and linking working in the CMakeLists.txt for DifferentialGeometry.
Comment 5 Ankur Sinha (FranciscoD) 2011-07-20 10:43:12 EDT
Hi Richard,

(In reply to comment #4)
> I've made some progress trying to get it to build against the OpenNL package
> but I can't really test it because of:
> 
> rpmbuild -bc rpmbuild/vmtk/SPECS/vmtk.spec
> error: Failed build dependencies:
>         InsightToolkit-devel is needed by vmtk-0.9.0-2.fc14.x86_64
>         vxl-devel is needed by vmtk-0.9.0-2.fc14.x86_64
> 
> Because of that I've added the review request of vxl and InsightToolkit as a
> blocker for this Review Request.

I'm sorry, I forgot to mention that you need to build ITK and vxl for your fedora system before you can test it. I for example, have a local mock repo with these packages. 

[root@ankur ~]# ls /tmp/mock-repo/InsightToolkit-* /tmp/mock-repo/vxl-*
/tmp/mock-repo/InsightToolkit-3.20.0-5.fc16.i686.rpm            /tmp/mock-repo/vxl-1.14.0-1.fc16.i686.rpm
/tmp/mock-repo/InsightToolkit-3.20.0-5.fc16.src.rpm             /tmp/mock-repo/vxl-1.14.0-1.fc16.src.rpm
/tmp/mock-repo/InsightToolkit-debuginfo-3.20.0-5.fc16.i686.rpm  /tmp/mock-repo/vxl-debuginfo-1.14.0-1.fc16.i686.rpm
/tmp/mock-repo/InsightToolkit-devel-3.20.0-5.fc16.i686.rpm      /tmp/mock-repo/vxl-devel-1.14.0-1.fc16.i686.rpm
/tmp/mock-repo/InsightToolkit-doc-3.20.0-5.fc16.i686.rpm        /tmp/mock-repo/vxl-doc-1.14.0-1.fc16.i686.rpm
/tmp/mock-repo/InsightToolkit-examples-3.20.0-5.fc16.i686.rpm



> 
> Back to this review :)
> 
> The only part of the project that appears to use OpenNL is the
> DifferentialGeometry sub-project. 
> 
> I've added a cmake option, USE_EXTERN_OPENNL to the main CMakeLists.txt and I'm
> working on getting the headers and linking working in the CMakeLists.txt for
> DifferentialGeometry.


The package works with the fedora OpenNL which I packaged recently. You don't need to make any tweaks. The changes I've made in the prep and build sections are enough. 

At the moment, gdcm is broken in rawhide,(after the latest libpoppler soname bump) so I can't build it as well :/

Thanks,
Ankur
Comment 6 Richard Shaw 2011-07-20 11:43:55 EDT
(In reply to comment #5)
> (In reply to comment #4)
> > I've made some progress trying to get it to build against the OpenNL package
> > but I can't really test it because of:
> > 
> > rpmbuild -bc rpmbuild/vmtk/SPECS/vmtk.spec
> > error: Failed build dependencies:
> >         InsightToolkit-devel is needed by vmtk-0.9.0-2.fc14.x86_64
> >         vxl-devel is needed by vmtk-0.9.0-2.fc14.x86_64
> > 
> > Because of that I've added the review request of vxl and InsightToolkit as a
> > blocker for this Review Request.
> 
> I'm sorry, I forgot to mention that you need to build ITK and vxl for your
> fedora system before you can test it. I for example, have a local mock repo
> with these packages. 

Yup, I run a local repo as well. I'll try that.


> > The only part of the project that appears to use OpenNL is the
> > DifferentialGeometry sub-project. 
> > 
> > I've added a cmake option, USE_EXTERN_OPENNL to the main CMakeLists.txt and I'm
> > working on getting the headers and linking working in the CMakeLists.txt for
> > DifferentialGeometry.
> 
> 
> The package works with the fedora OpenNL which I packaged recently. You don't
> need to make any tweaks. The changes I've made in the prep and build sections
> are enough. 

Well, I was trying for a few reasons. One, I'm trying to improve my cmake-fu :) Also, hacking the link.txt is not ideal. If I can come up with a good cmake patch then maybe upstream will accept it (which is the best case). 

Also, in retrospect, I'm wondering if we should have renamed the header file for OpenNL to be consistent with the library name, i.e.: nl.h -> opennl.h

What do you think?

Richard
Comment 7 Richard Shaw 2011-07-21 10:51:19 EDT
Ok, after installing the other BuildRequires I got my cmake patch working!

Next issue: 
Are you on a 32bit or 64bit system? I noticed that "make install" doesn't put the libraries in /usr/lib64 on x86_64 systems. I added a %ifarch conditional to move /usr/lib to /usr/lib64 on x86_64 and ppc64 systems.

Second issue:
I noticed it put all the python files in /usr/lib/vmtk/vmtk. I think all of these should go into %{python_sitelib}/vmtk, right?

Richard
Comment 8 Richard Shaw 2011-07-21 12:11:02 EDT
Update:

I need your help to figure out what exactly is supposed to go into the -devel package. 

I saw you had the python scripts going in there. Are they the "source" files for the python libraries or are they separate from them?

I find the whole /usr/lib/vmtk/vmtk structure that make install produces confusing...
Comment 9 Ankur Sinha (FranciscoD) 2011-07-21 14:11:47 EDT
> 
> Well, I was trying for a few reasons. One, I'm trying to improve my cmake-fu :)
> Also, hacking the link.txt is not ideal. If I can come up with a good cmake
> patch then maybe upstream will accept it (which is the best case). 
> 
> Also, in retrospect, I'm wondering if we should have renamed the header file
> for OpenNL to be consistent with the library name, i.e.: nl.h -> opennl.h
> 
> What do you think?
> 

Hrm, we certainly could do that. Although, we'd have to first look at it's effects. I'll look into it.
Comment 10 Ankur Sinha (FranciscoD) 2011-07-21 14:19:33 EDT
(In reply to comment #7)
> Ok, after installing the other BuildRequires I got my cmake patch working!
> 
> Next issue: 
> Are you on a 32bit or 64bit system? I noticed that "make install" doesn't put
> the libraries in /usr/lib64 on x86_64 systems. I added a %ifarch conditional to
> move /usr/lib to /usr/lib64 on x86_64 and ppc64 systems.

I run mock builds for a 32 bit system. I haven't tried on an x86_64 yet. 

I just looked at CMakeLists.txt. I think setting "VMTK_MODULE_INSTALL_LIB_DIR" would fix this. -DVMTK_MODULE_INSTALL_LIB_DIR:STRING=$RPM_BUILD_ROOT/%{_libdir}/ or something on these lines. Again, I'll try it out and confirm.

> 
> Second issue:
> I noticed it put all the python files in /usr/lib/vmtk/vmtk. I think all of
> these should go into %{python_sitelib}/vmtk, right?

Yes. Should. Again, setting VMTK_MODULE_INSTALL_LIB_DIR just might do the trick.

I'll try all of this out and post :)

Thanks,
Ankur
Comment 11 Ankur Sinha (FranciscoD) 2011-07-21 14:21:05 EDT
(In reply to comment #8)
> Update:
> 
> I need your help to figure out what exactly is supposed to go into the -devel
> package. 
> 
> I saw you had the python scripts going in there. Are they the "source" files
> for the python libraries or are they separate from them?
> 
> I find the whole /usr/lib/vmtk/vmtk structure that make install produces
> confusing...

I'll try getting in touch with upstream for suggestions.

I'm going to be unavailable until Monday. I'll only respond after that. I regret the inconvenience this will cause :/

Thanks,
Ankur
Comment 12 Richard Shaw 2011-07-21 15:43:16 EDT
(In reply to comment #10)
> (In reply to comment #7)
> > Ok, after installing the other BuildRequires I got my cmake patch working!
> > 
> > Next issue: 
> > Are you on a 32bit or 64bit system? I noticed that "make install" doesn't put
> > the libraries in /usr/lib64 on x86_64 systems. I added a %ifarch conditional to
> > move /usr/lib to /usr/lib64 on x86_64 and ppc64 systems.
> 
> I run mock builds for a 32 bit system. I haven't tried on an x86_64 yet. 

Ok, my updated spec/SRPM file[1] fixes that.


> I just looked at CMakeLists.txt. I think setting "VMTK_MODULE_INSTALL_LIB_DIR"
> would fix this.
> -DVMTK_MODULE_INSTALL_LIB_DIR:STRING=$RPM_BUILD_ROOT/%{_libdir}/ or something
> on these lines. Again, I'll try it out and confirm.

Didn't work for me. It appears to just control the /usr or /usr/local part of the path, I set it to /lib64 (since it adds /usr seperately) and it ended up putting the files in /usr/lib64/lib/vmtk/vmtk... 



> > Second issue:
> > I noticed it put all the python files in /usr/lib/vmtk/vmtk. I think all of
> > these should go into %{python_sitelib}/vmtk, right?
> 
> Yes. Should. Again, setting VMTK_MODULE_INSTALL_LIB_DIR just might do the
> trick.

Probably will not work but worth a try... The problem is all the .py files need to go in %{python_sitelib} which is /usr/lib/pythonX.X/site-packages/ while the .so python libraries need to go in %{python_sitearch} which can be the same as %{python_sitelib} for 32bit systems but is /usr/lib64... for 64 bit systems. This is easier just to manage manually.

The only side effect I've found so far is that one (and only one) of the python .so libraries actually has a SONAME and the .so symlink is getting pulled into the regular package instead of the -devel package. Should be easy to fix though, there are other bigger problems...

One of the "big" problems is actually with vtk, not vmtk. It provides a necessary library, libVPIC.so, but only in the -devel package, not in the main package. Because of that I had to add a Requires for vtk-devel which pulls in a bunch of dependencies we don't need just to get the one file we do need. I added a BZ reference in the comment so we can watch to see when it get's fixed.

The other problem seems to be that some of the libraries from vmtk can't find other libraries it provides. In other words, it's finding libraries in /usr/lib{,64} but not the ones in /usr/lib{,64}/vmtk... Not sure why that is happening.

That's enough for now I think!

[1] http://hobbes1069.fedorapeople.org/vmtk/vmtk-0.9.0-2.fc14.src.rpm
Comment 13 Richard Shaw 2011-07-26 14:27:21 EDT
Ankur,

I found out that I was wrong about the whole python_sitelib/sitearch thing. If there are ANY C python modules in the package then everything, including the pure python modules go in sitearch.

Richard
Comment 14 Ankur Sinha (FranciscoD) 2011-08-13 03:30:55 EDT
hi Richard, 

I'm waiting on vxl to be packaged. Mario's working on it. I can't get vmtk to build in mock at the moment. Some issue with the vxl package. 

Thanks,
Ankur
Comment 15 Miroslav Suchý 2012-12-16 08:21:04 EST
Ping? Any progress here? Or we can close this review?
Comment 16 Ankur Sinha (FranciscoD) 2012-12-16 22:12:50 EST
Hi,

VXL has *just* been included in the repositories. I'm away and will not be able to work on this review for another month. Please feel free to start a new review if you need to.

Thanks,
Warm regards,
Ankur
Comment 17 Ankur Sinha (FranciscoD) 2013-01-13 20:40:38 EST
Hrm, cannot still build it. Depends on ITK. Also found more bundled software *sigh* in the 1.0.1 release:

Doxygen  OpenNL  Stellar_1.0  tetgen1.4.3. I also see that tetgen is NOT free software, as noted in the review ticket:

https://bugzilla.redhat.com/show_bug.cgi?id=483663

I'm afraid this might mean we need to drop this package altogether. I'll see if I can speak to upstream about this.

I'll at least package stellar in the mean time: http://www.cs.berkeley.edu/~jrs/stellar/

Thanks,
Warm regards,
Ankur
Comment 18 Mario Ceresa 2013-01-14 04:42:50 EST
Thanks Ankur for your efforts in packaging vmtk. You might be able to use vmtk with netgen, which is free:
http://www.vmtk.org/Tutorials/NetgenWay/

We hope to finish ITK anytime soon -if more bundled software doesn't show up, of course ;)

Best,
Mario
Comment 19 Dominik 'Rathann' Mierzejewski 2014-06-24 08:03:03 EDT
tetgen is free software now and I've just approved Sandro's package, so it can be unbundled from vmtk.

Note You need to log in before you can comment on or make changes to this bug.