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

[MergeTree*] Clean and optimize code #923

Merged
merged 2 commits into from
Apr 21, 2023

Conversation

MatPont
Copy link
Contributor

@MatPont MatPont commented Apr 19, 2023

This PR cleans and optimizes the code related to merge tree classes like distance, barycenter and clustering.

  • Regarding cleaning:

    • It removes the "rescaled Wasserstein" option, it was an attempt to alleviate the normalization by rescaling using the values of the other tree (when computing a distance). Unfortunately this is not a metric, the option is not here anymore on the ParaView GUI and it was never really used so I think it makes sense to remove it.
    • It removes the "Normalized Wasserstein regularizer", another attempt to alleviate normalization. With the same arguments than before I think it should be removed.
    • It removes the attempt to progressively compute the distance between merge trees, the corresponding code related to progressivity is not even in TTK but some variables and condition still remains in the code that I think should be removed.
  • Regarding optimization:

    • Fix an implicit conversion between float and double that could cause a round-off error hence a non determinism depending on the machine executing the code.
    • Merge tree distance now call the exhaustive solver when the assignment problem is small and have few combinations to test.
    • Some mathematical terms were removed in the Wasserstein distance compared to the classical edit distance (that uses the minimum of three terms for computing distance between trees and between forests). Even if the Wasserstein distance does not use these terms, they were still computed, this PR avoid the computation of these terms when the Wasserstein distance backend is choosen.
  • Regarding new features:

    • Prepare the code for the "Convert to diagram" option, that convert the merge trees given in input of the filter to diagrams. It allows the user to give merge trees and ask to consider them as diagrams hence having diagrams visualization instead of merge trees visualisation. It is particularly useful to have the same pipeline and just change an option to switch between diagrams and merge trees computation and visualization (before this PR we could only give merge trees or persistence diagrams, and not convert merge trees to persistence diagrams).
    • It fixes the persistence thresholding of merge trees. From a pure practical point of view, a BDT should contains at least two nodes (to have at least one edge), so we need to keep at least two persistence pairs. If there is only one non-zero persistence pair we need to add a "dummy" persistence pair with zero persistence (that will be matched to the diagonal with zero cost when computing a distance). It is a very special case.

MatPont added a commit to MatPont/ttk-data that referenced this pull request Apr 20, 2023
MatPont added a commit to MatPont/ttk-data that referenced this pull request Apr 20, 2023
MatPont added a commit to MatPont/ttk-data that referenced this pull request Apr 20, 2023
@julien-tierny
Copy link
Collaborator

ok, looks good. thanks!

@julien-tierny julien-tierny merged commit b4d2ea8 into topology-tool-kit:dev Apr 21, 2023
@MatPont MatPont deleted the mergeTreeClean branch April 24, 2023 07:31
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

Successfully merging this pull request may close these issues.

2 participants