SortingAnalyzer: only compute what is necessary

[JulienBrn]

Hi, my understanding is that the sorting analyzer is aware of dependencies between extensions and handles lazyness of what gets computed. However, I'm having trouble using this in a straightforward way and the code to get there seems more and more convoluted and I believe I'm misusing the analyzer.

My goal

when given an empty analyzer (no extensions computed), I simply use (for an input dictionary d)

analyzer.compute_several_extensions(
  {k:d.get(k, {}) for k in analzer.get_computable_extensions() if not d.get(k, True) == False}
)

However, I want to achieve the exact same result (up to random sampling) using an analyzer where some extensions may have already computed, possibly with the same parameters (in which case we aim to not recompute it) or with different parameters (in which case we need to recompute that extension and all those depending on it).

The problem

It seems that compute forces recomputing of the extension even if it has the exact same parameters. Thus one needs to check manually if that extension is already in the analyzer with the same parameters. This is a hard task because analyzer.get_default_extension_params(k) | d[k] seems slightly different then analyzer.get_extension(k).params. For example waveform has 'dtype': None in the former and 'dtype': '<f4' in the latter, templates has 'operators': None in the former and 'operators': ['average', 'std'] in the latter, ...

Is there an easy manner of achieving what I want ?

My code

The current code to achieve something close to what I want (but depending on how exactly dependencies are managed in spikeinterface, it may not be right) is the following. Is this a correct way of achieving my goal ?

analyzer = si.load("my_previous_analyzer_path").copy()
analyzer.set_temporary_recording(rec) #To handle the case where the recording changed path
analyzer.sorting = sorting #To handle the case where the sorting changed path

#For this issue I've put the config file as a string
config_str= """
random_spikes:
  method: "uniform"
  max_spikes_per_unit: 400
waveforms:
    ms_before: 1
    ms_after: 2
amplitude_scalings: False
spike_locations: False
"""

params=yaml.safe_load(config_str)
real_params = {k:(analyzer.get_default_extension_params(k) | params.get(k, {})) 
 for k in analyzer.get_computable_extensions() if params.get(k, {})!=False}

extensions_to_compute = {}
for k in analyzer.get_computable_extensions():
    if not k in real_params:
      if analyzer.has_extension(k):
        analyzer.delete_extension(k)
    elif not analyzer.has_extension(k):
       extensions_to_compute[k] = real_params[k]
    elif real_params[k] != analyzer.get_extension(k).params:
      diff = {p:(real_params[k][p], analyzer.get_extension(k).params.get(p, None)) 
                   for p in real_params[k] if not real_params[k][p] is None}
      diff = {k:(v1, v2) for k, (v1, v2) in diff.items() if v1!=v2 and not v2 is None}
      if len(diff) > 0:
        print(k, diff)
        extensions_to_compute[k] = real_params[k]

analyzer.compute_several_extensions(extensions_to_compute)

Additionally, it would be really nice to have an option to analyzer.compute_several_extensions in order to keep computing extensions when there is an error (for those that do not depend on the errored extension).

[samuelgarcia]

Hi, my understanding is that the sorting analyzer is aware of dependencies between extensions and handles lazyness of what gets computed. However, I'm having trouble using this in a straightforward way and the code to get there seems more and more convoluted and I believe I'm misusing the analyzer.

Not exactly. There is not lazyness/caching of what is computed.
When you compute something : this something is re computed always.
The dependencies graph is mostly to trigger an automatic deletion of extensions B, C, D that depend on an extension A when extension A is re-computed (with different params or not). This is because B, C and D could change there reults fater A recomputation. (typically quality metrics depend on many other extension and we when to autodelete then if you recompute templates for instance or correlograms)

[zm711]

A couple additional comments.

The analyzer caches its own sorting. So the sorting path is not important. You do not need to change that. That's why we have a set_temp_recording function. Because we can't cache the recording due to size. But sorting are small enough. So I would delete that line.

It seems that compute forces recomputing of the extension even if it has the exact same parameters. Thus one needs to check manually if that extension is already in the analyzer with the same parameters.

We discussed this idea. I believe @chrishalcrow was trying to implement this but it was actually much more complicated to do it correctly. Maybe he can comment on the struggles with this. (Unless it was someone else who tried to implement it). So it is not trivial.

Additionally, it would be really nice to have an option to analyzer.compute_several_extensions in order to keep computing extensions when there is an error (for those that do not depend on the errored extension).

I think that is a cool idea, but I do think this will require pretty complicated logical to recheck the dependency graph. It has been a while since I've looked at this code myself, so I honestly don't remember where we would have to recheck that info in the case of an error.

[chrishalcrow]

Hello, I implemented something like this at the quality/template metric level. Now if the parameters are the same, and you're just computing metrics, we don't recompute any that were previously computed.

I think the ideas suggested here are pretty easy to implement cause we have a great graph extension structure, but it's hard to agree on what we want/expect the behaviour to be and what the syntax should be. As Sam says, early on we agreed on "When you compute something : this something is re computed always" to try and be safe, since it's hard to ensure that everything really is the same as before (the recording, sparsity, all dependent extensions, ...). But might be good to have a wider discussion on this point.

[zm711]

I do remember that discussion now. Yeah I think the fact you can put on a temp recording means that even the exact same params it is hard for us to guarantee that we shouldn't recompute. I think compute should compute was the right idea.

[JulienBrn]

I do remember that discussion now. Yeah I think the fact you can put on a temp recording means that even the exact same params it is hard for us to guarantee that we shouldn't recompute. I think compute should compute was the right idea.

We could assume the recording is the same, or store some hash of the recording just to make sure.

I think the ideas suggested here are pretty easy to implement cause we have a great graph extension structure

Yes, but for the fact that get_default_extension_params returns something different than the actual params stored in the extension...

but it's hard to agree on what we want/expect the behaviour to be and what the syntax should be

Wouldn't a function like compute_necessary_extensions(list of extensions, parameter_for_those_extensions) whose semantic is the following fairly clear?

• the resulting sorting analyzer object is equivalent (up to some random seed) to removing all extensions and launching compute_several_extensions
• however, it may not (re)compute all extensions, only those that are missing or require recomputation because they have changed parameters or depend on extensions that need recomputing.

[zm711]

@alejoe91 you haven't weighed in yet :)

[alejoe91]

Thanks for pinging @zm711

I think we could achieve both behaviors with an extra force_recompute parameter (True by default).
When False, we could check that the extension is computed and params are the same. The workaround not to use default params is to instantiate a new extension object with the provided params and check the new params against the old ones.

What do you guys think?