Add ultrasound confidence map to transforms
Description
This transform uses the method introduced by Karamalis et al. in https://doi.org/10.1016/j.media.2012.07.005 to compute confidence maps on ultrasound images.
Possible Problems
-
I am not entirely sure if the "transforms" section is the right place for this method but I found it the most suitable since it is not "deep learning" and it is "pre-processing" in a way.
-
Current version of the implementation requires GNU Octave to be installed and defined in the path. This is an odd dependency, I am aware of that, yet using SciPy does not provide satisfactory results in terms of speed. If this kind of dependency is not suitable, I also have a pure SciPy implementation, yet it runs about x15 slower, and it is slow to work in real-time, I am open to any feedback.
Types of changes
- [x] Non-breaking change (fix or new feature that would not break existing functionality).
- [ ] Breaking change (fix or new feature that would cause existing functionality to change).
- [ ] New tests added to cover the changes.
- [ ] Integration tests passed locally by running
./runtests.sh -f -u --net --coverage. - [ ] Quick tests passed locally by running
./runtests.sh --quick --unittests --disttests. - [ ] In-line docstrings updated.
- [ ] Documentation updated, tested
make htmlcommand in thedocs/folder.
thanks for the PR! for the two concerns:
- perhaps the relevant functions could be moved to
monai.dataand only leave a thin wrapper inmonai.transforms, so that the users can access the core functions directly without having to understandmonai.transformsAPIs. - I think it's fine to currently depend on GNU Octave, however we won't ship/redistribute Octave, so would be great to provide some instructions in the docstring about how to ensure necessary environment to run the components. or optionally, the component could automatically detect octave/scipy and run with the corresponding implementations. Including the scipy implementation will also make the unit testing easier because it's available in the testing environments.
thanks for the PR! for the two concerns:
- perhaps the relevant functions could be moved to
monai.dataand only leave a thin wrapper inmonai.transforms, so that the users can access the core functions directly without having to understandmonai.transformsAPIs.- I think it's fine to currently depend on GNU Octave, however we won't ship/redistribute Octave, so would be great to provide some instructions in the docstring about how to ensure necessary environment to run the components. or optionally, the component could automatically detect octave/scipy and run with the corresponding implementations. Including the scipy implementation will also make the unit testing easier because it's available in the testing environments.
Thanks for the great feedback! I'll get to it ASAP.
Hi @MrGranddy, looks like a good addition and moving function elsewhere and using the transform as a wrapper is a good idea. However I don't think keeping Octave here is a good idea, it's a large dependency and probably isn't very fast. I'd also suggest not relying on scipy so that people can avoid having to install it as well. The fewer dependencies we have the fewer issues with restricted environments we'll encounter.
In general I think your code can be converted to be all Pytorch and avoid using Numpy/Octave entirely. The mldivide function I believe can be done with torch.linalg.solve (or at least is close enough), we have a hilbert transform layer already, resample layers to avoid using OpenCV, and otherwise use Pytorch equivalents in places. The use of csc_matrix I think we can just get around with a different structure. Using Pytorch only (or switch to a from numpy is absolutely needed) would be much faster and more coherent, we could even consider doing this calculation on GPU in that case. I would suggest porting your algorithm to Pytorch then for your next submission.
Hi @MrGranddy, looks like a good addition and moving function elsewhere and using the transform as a wrapper is a good idea. However I don't think keeping Octave here is a good idea, it's a large dependency and probably isn't very fast. I'd also suggest not relying on scipy so that people can avoid having to install it as well. The fewer dependencies we have the fewer issues with restricted environments we'll encounter.
In general I think your code can be converted to be all Pytorch and avoid using Numpy/Octave entirely. The
mldividefunction I believe can be done withtorch.linalg.solve(or at least is close enough), we have a hilbert transform layer already, resample layers to avoid using OpenCV, and otherwise use Pytorch equivalents in places. The use of csc_matrix I think we can just get around with a different structure. Using Pytorch only (or switch to a from numpy is absolutely needed) would be much faster and more coherent, we could even consider doing this calculation on GPU in that case. I would suggest porting your algorithm to Pytorch then for your next submission.
Hello, the original implementation actually does exactly what you describe, but it works really slow due to the nature of the algorithm, SciPy sparse solvers are not as fast as the Octave one, PyTorch is even slower. So the main problem here is that the problem solves for a sparse matrix, and that matrix gets really big really fast since it solves for the adjacency matrix of an image. NxN image means N^2 x N^2 matrix. PyTorch sparse library is very underdeveloped as far as I know, and SciPy sparse solver is 15x times slower compared to the Octave solver, at least on Windows, there is the possibility Linux binaries are better but I couldn't test it on a Linux machine. I also tried stochastic solvers like conjugate gradients, they are fast but they do not produce good results.
I also closed the PR by accident, can you please open it again :) Thank you. Looking forward to your feedback as always :)
Hi @MrGranddy I see the problem then a little better. The issue though is the process for installing Octave and oct2py is more complex than other things we rely on, requiring a separate install and a correctly configured PATH variable. This is also an issue for our CICD system since we do need to test this transform. The library you're using, oct2py, also seems like a rather small project whose long term support is not assured. I'd still recommend we look into some other solver (something wrapping eigen?) to avoid this dependency.
Hi @MrGranddy I see the problem then a little better. The issue though is the process for installing Octave and oct2py is more complex than other things we rely on, requiring a separate install and a correctly configured PATH variable. This is also an issue for our CICD system since we do need to test this transform. The library you're using, oct2py, also seems like a rather small project whose long term support is not assured. I'd still recommend we look into some other solver (something wrapping eigen?) to avoid this dependency.
Hello, for now I've removed the octave dependency, now the code only uses scipy solver. As far as my knowledge goes, there is no decomposition or iterative method that would make the algorithm faster while preserving the results. I am also sharing some metrics for the comparison of octave and scipy. I believe SciPy is not all bad, considering they also would optimize their sparse solver over time.
For an image with 118677 pixels (around 350 x 350), using i7-13700K with 10 runs each:
Scipy mean time: 5.763958525657654 +- 0.2072540601999276
Octave mean time: 0.7772286891937256 +- 0.018855939984020915
/build
/build