Open MarcMachaczek opened 2 years ago
@MandMarc thanks for the input! This could be interesting, perhaps it is only a matter of adding appropriate bindings to the C++ code. Both @MonkeyBreaker and I are very busy these days so we ca''t give you a timeframe if we have to do it ourselves but of course we accept PRs and we could try to help you if you'd like to try to add this feature yourself.
Yes, I thought so too since the functionality technically already exists! Actually, I'm pretty interested in working on bindings etc. However, I have never done this before and don't have too much experience in C++ (a little tho), so this would require quite some time on my side which I won't have before christmas holidays unfortunately. Nonetheless, could you by any chance recommend some material/references regarding the whole Python C++ binding topic?
Hi @MandMarc,
Thank you for interest in this feature. I need to be honest, I did not touch the filtration generation at all, meaning I don't have much experience on this.
But, to explain how the filtration are generated for the moment is as follow:
flagser
algorithms.math
that describes the filtration in a kind of pseudo-code.build_custom_algorithms.sh
algorithms.math
pseudo-code into flagser/include/algorithms.h
c code.pyflagser
from the root of the repository with python -m pip install -e .
.I never tried to create a new filtration, so I don't know much of the pseudo-code syntax. But the rest, I could help in debug if necessary.
As you can understand, the filtration are hard-coded into the code. And if you want to add a new filtration, you are required to compile again the package. What should be necessary, is to add the possibility from Python to pass a filtration function.
In order to achieve this, we should define a common interface for filtration that matches what flagser
expects.
I think it is definitely doable, but as @ulupo said, currently we are really busy, I cannot give a specific time frame.
I can provide by end of the week, what I think the changes in the different parts of the code should be and if you are interested, you can start from there, otherwise, please let me some time to be able to implement this.
Best, Julián
Hi again,
I give it a second though about the feature.
The filtration is a core part of the algorithm. This function is called extensively.
If we would want to provide a custom filtration, we would need to provide a callback function from Python to the C++ backend.
This is definitely doable for pybind11, but I am afraid about the performances implication. I opened a discussion in pybind11
about this, to see if I am right in thinking that performance will be highly impacted, or if I am wrong.
The reason I think this could have a big runtime impact is because, if I understood right the documentation, when you provide from Python a callback. Then in the C++ core, it will call the Python function and then the Python function will return the value to the C++ core. Hope I am clear in my explanation.
I will wait on their input to see if we should proceed by providing a callback from Python directly.
Otherwise, a different approach, would be to add to the bindings the possibility to add new filtrations. But even this will require to compile again the package.
I cannot see a good solution (good in term of performance and easy to use), but I will continue to think about this.
@MandMarc I will try to keep you updated and not delay to much this, but It could take time to have a smart solution. If on the meantime you can try the solution I posted before, I can provide you assistance.
In the original Flagser documentation https://github.com/luetge/flagser/blob/master/docs/documentation_flagser.pdf you can find the option to add custom filtration algorithms. Would it be possible to implement this by e.g. passing a string or rather a .txt file in the format of the mathematical input language used in flagser?