search

proteneer / timemachine

Differentiate all the things!
Other
138 stars 17 forks source link

Add atom mapping function returning MCS diagnostics #1231

Closed mcwitt closed 9 months ago

mcwitt commented 9 months ago

Adds atom_mapping.get_cores_and_diagnostics having the same signature as get_cores except that it additionally returns an MCSDiagnostics object containing diagnostic information collected during the MCS search.

Useful for benchmarking.

badisa commented 9 months ago

On a related note nodes_visited from MCSResult is reset between core size, @maxentile and I were talking about this offline today. So the nodes_visited could be reduced significantly without actually reducing the computation time, IE a change to the implementation results in more nodes being visited overall, but skips to a smaller core size and selects a mapping with a minimal number of visits and returns some tiny number of visited nodes.

Made a branch that reset the max visits, warrants more investigation before doing anything with it https://github.com/proteneer/timemachine/tree/task/dont-reset-visited-nodes

mcwitt commented 9 months ago

On a related note nodes_visited from MCSResult is reset between core size, @maxentile and I were talking about this offline today. So the nodes_visited could be reduced significantly without actually reducing the computation time, IE a change to the implementation results in more nodes being visited overall, but skips to a smaller core size and selects a mapping with a minimal number of visits and returns some tiny number of visited nodes.

Made a branch that reset the max visits, warrants more investigation before doing anything with it https://github.com/proteneer/timemachine/tree/task/dont-reset-visited-nodes

Good point, I hadn't realized this. In https://github.com/proteneer/timemachine/pull/1231/commits/4cde9b927cbd34fb667e204fc9224803700f1b8b I modified the code to return the total number of nodes visited instead of the MCSResult object, since we're mainly interested in the total nodes visited as a proxy for effort, and it seemed potentially confusing to mix size-specific data in MCSResult with a global nodes_visited.