search

masadcv / FastGeodis

Fast Implementation of Generalised Geodesic Distance Transform for CPU (OpenMP) and GPU (CUDA)
https://fastgeodis.readthedocs.io
BSD 3-Clause "New" or "Revised" License
91 stars 14 forks source link

build error with GeodisTK (required for examples) #14

Closed jacobmerson closed 2 years ago

jacobmerson commented 2 years ago

I'm working through the Joss review (https://github.com/openjournals/joss-reviews/issues/4532) and I am trying to run some of the samples scripts. These alll require GeodisTK which has build errors on Clang13.0.1.

I have opened an issue on the GeodisTK repo here: https://github.com/taigw/GeodisTK/issues/14

masadcv commented 2 years ago

Hi @jacobmerson , Many thanks for reporting this, I will look into this..

GeodisTK is only used for baseline comparisons and hence (if needed) can be safely removed from samples

jacobmerson commented 2 years ago

@masadcv I submitted a pull request to GeodisTK with the fixes for those errors. https://github.com/taigw/GeodisTK/pull/15

masadcv commented 2 years ago

@jacobmerson many thanks!

I have opened a PR (#18 ) to add a comparison Geodesic distance method that we can use for comparisons in samples and hence become independent of external GeodisTK package.

This is still a WIP, but I will try to get this in during the weekend..

masadcv commented 2 years ago

Apologies as this was automatically closed, reopening.

As mentioned in my comment above, I have implemented Toivanen's Geodesic method within FastGeodis package. This is a comparison existing algorithm that was compared to with in GeodisTK. We dont have any dependency on GeodisTK anymore.

Can I please ask you to update to rc7 version and trying the failing samples again? If all looks good, you can go ahead close this issue. Otherwise, please let me know if you see anything missing/not working.

jacobmerson commented 2 years ago

The pull request in GeodisTK got merged (and you provided a stand alone version) so I'm going to go ahead and close this.