Skip to content

Conversation

@thom-dani
Copy link
Contributor

@thom-dani thom-dani commented Apr 15, 2025

Thanks for contributing to TTK!

Before submitting your pull request, please:

  • Review our Contributor Guidelines, in particular regarding code formatting (with clang-format) and continuous integration.

  • Please provide a quick description of your contributions below:

Implementation of stochastic discrete gradient.

@julien-tierny
Copy link
Collaborator

TODO:

  • add a second template parameter to the discrete gradient toolbox, with a default value (that will prevent a break in compatibility with pre-existing calling code)

@julien-tierny
Copy link
Collaborator

  • perf check wrt tdev: OK!
  • default backend selection: OK!

@julien-tierny
Copy link
Collaborator

julien-tierny commented Nov 24, 2025

@thom-dani bug report

  • in the paraview gui, open a dataset,
  • ask to compute the morse-smale complex with the stochastic gradient
  • change the seed for the stochastic gradient
  • nothing happens (i.e., the separatrices don't change wrt the first computation)

the problem is that there is a gradient caching mechanism and that mechanism needs to be disabled if the gradient parameters (backend, seed) change from one execution to the next.

thanks for fixing this ASAP and letting me know when this is done.

@julien-tierny
Copy link
Collaborator

hi @thom-dani , thanks for the hot fix.
there's still an issue with the way gradient cache is handled.
consider the following use case:

  • compute the morse smale complex with the homotopic expansion, click on apply
  • modify the backend to the stochastic gradient, click on apply
  • now click on the button "Execute" (the backend hasn't change, nor the seed, so we use the cached gradient) ==> the code uses the gradient from the homotopic expansion.
    are you sure you store the stochastic gradient to cache?

@thom-dani
Copy link
Contributor Author

thom-dani commented Dec 1, 2025

Hello @julien-tierny, I fixed the bug and I don't observe the behavior you described anymore.

@julien-tierny
Copy link
Collaborator

alright, the bug is fixed. The features are working as expected now!

@julien-tierny
Copy link
Collaborator

new bug report:

  • run the example from ttk-data states/cosmicWeb.pvsm
  • select the object "MorseSmaleComplex2" and switch from homotopic expansion to stochastic gradient
    => segfault

std::vector<char> isPL;
this->discreteGradient_.getCriticalPointMap(criticalPoints, isPL);

// dmt1Saddle2PL_.resize(triangulation.getNumberOfEdges());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thom-dani
can these comments go away?

Timer t;

const bool allowBoundary = true;
// const bool returnSaddleConnectors = false;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thom-dani
can these comments go away?

isRemovableSaddle.resize(numberOfEdges);

std::vector<int> dmt1Saddle2PL_(numberOfEdges, -1);
// dmt1Saddle2PL_.resize(numberOfEdges);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thom-dani
can't these comments go away?

isRemovableSaddle.resize(numberOfTriangles);

std::vector<int> dmt2Saddle2PL_(numberOfTriangles, -1);
// dmt2Saddle2PL_.resize(numberOfTriangles);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thom-dani
can't these comments go away?


if(vpath.isValid_) {
// add persistence pair to collection if necessary
// if(CollectPersistencePairs and outputPersistencePairs_) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comments to remove as well.

@julien-tierny
Copy link
Collaborator

ok, the code looks good.
TODOs

  • remove unnecessary comments
  • fix backend switch crash (cosmicWeb.pvsm)

@thom-dani thom-dani force-pushed the stochastic-discrete-gradient branch from faac9a4 to 9c5a463 Compare December 4, 2025 10:36
@julien-tierny
Copy link
Collaborator

julien-tierny commented Dec 31, 2025

everything ok.

TODO:
[x] check topologicalOptimization_darkSky_postProcess: issue on that example, with dev too?.
[x] waiting for the CI to finish

@julien-tierny
Copy link
Collaborator

alright, let's go!

@julien-tierny julien-tierny merged commit 98e6984 into topology-tool-kit:dev Jan 1, 2026
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants