Optimize prewarming for container slimmable models

[sdatkinson]

Don't need to prewarm all of the models--just the active one.

On SetSlimmableSize, prewarm the model if you need to use a different one.

[rerdavies]

I was arriving to file a request on this issue, and here it is!

Although I think I have to object to the proposed solution. Just prewarming if a submodel changes does not solve the deeper underlying problem.

To get a working NAM instance with the correct active submodel after loading a preset, or after a change to the plugins' SlimmableSize control, current code requires EIGHT calls to prewarm(), the majority taking place on the larger submodel. On lightweight machines this enough to destabilize the entire system. And it's a problem on heavyweight machines, where users have added a large number of NAM instances. There is no easy way to deal with this kind of load in a realtime system. Either, the prewarming has to be performed serially (with 3 or 4 second preset load times), or you run the risk of completely pinning all CPUs on high priority threads for a substantial portion of second (which Ubuntu particularly dislikes). Changes to NeuralModel code could reduce it to six prewarm() calls; and you could easily remove one more by removing the prewarm() call on the active submodel in Container::prewarm(). But I would be happier if two unnecessary calls to prewarm() on the large model could be avoided.

NAM Core itself makes three prewarm() calls in get_dsp. And then requires the caller to make two more in order to Reset with correct sample rate and buffer size, and select the ACTUAL intended submodel. And not do what seems like the right thing and call Prewarm().

NAM Core could -- at a minimum -- reduce this to two prewarm() calls -- one on each of the submodels. A third prewarm() call gets made by Container::prewarm(), which calls prewarm() on the active submodel. This is unnecessary because both submodels have already been prewarmed(). All three calls get made on a line with the following comment:

 // "pre-warm" the model to settle initial conditions
 // Can this be removed now that it's part of Reset()?

Yes it could. :-)

I think you have two choices:

1. SetSlimmableSize is callable from a realtime thread. The Container prewarms both submodels, which means that SetSlimmableSize will never have to prewarm a new submodel on the realtime thread. During creation, two prewarm() calls are made.
2. SetSlimmable size is not callable from a realtime thread, because it may select a submodel that has not been prewarmed. During creation, only one prewarm() gets called, and callers get to choose which one.

I think the general solution is that none of the following methods call prewarm() either directly or indirectly.

• get_dsp()
• nam::SlimmableModel::SetSlimmableSize()
• -nam::DSP::Reset(...)

and that all internal Reset code, and prewarming calls get deferred until a mandatory call to nam::DSP::Prewarm().

This allows callers to make calls to SetSlimmabeSize() and Reset() between the get_dsp() call and the nam::DSP::Prewarm() call, and ensure that internal Reset and prewarm() operation get performed with caller-controlled values.

Whether the final Container::prewarm() call calls prewarm() on only the active submodel, or on both determines whether SetSlimmableSize can be called on a realtime thread. If one, then the caller has to go to a service thread because they have no idea whether a prewarm() is required; if both, thern there's no need to prewarm again, because both submodels have already been prewarmed.

My personal preference would be that SetSlimmableSize() not be callable on a realtime thread. (I'd rather have reduced prewarm time). But two prewarm() calls instead of 8 isn't a deal breaker. And being able to call SetSlimmableSize() on a realtime thread simplifies a LOT of code for me.

That being said, if you go with the non-realtime SetSlimmableSize(() option, you would be doing an enormous kindness if you could provide the following additional method on SetSlimmableSize():

bool WillSlimmableModelChange(float slimmableSize)

This would allow me to determine whether a subsequent call to SetSlimmableSize needs to be taken off the realtime thread. The implementation (for a Container) is easy: will the active model change? And one imagines that it would not be difficult to implement that for future creative uses made of the SetSlimmableSize API.

If you are looking for a non-breaking change in the short term, perhaps you could set a flag, and NOT perform prewarming for a particular model or submodel if it has already been prewarmed. Technical debt perhaps, because it's not as flexible and future-proof as a mandatory Prewarm() call would be. But it would solve my the immediate problem.

void XXX:prewarm()
{
     if (this->prewarmed) return;
     this->prewarmed = true;
}

[rerdavies]

If now isn't a convenient time, I may implement

void XXX:prewarm()
{
     if (this->prewarmed) return;
     this->prewarmed = true;
}

in a temporarily forked TooB version of NAM Core, just to get us through the next few weeks.

Can you think of a reason why this wouldn't work on the current implementation?

[mikeoliphant]

I'm running into problems with this, too - not so much the specific container behavior, but prewarming in general.

A model is getting prewarmed when:

• it is first loaded
• the max buffer size is changed (which can only be done through Reset - which calls prewarm even though it says it doesn't)
• the slim factor is changed in a way that needs updating

In my case (which I think is typical), all three of these operations happen at model load time, which results in a mad circus of redundant prewarming.

Possible solutions off the top of my head:

• make prewarming solely the responsibility of the caller (probably not ideal)
• allow setting global defaults for slim size and max buffer size
• take slim size and max buffer size as parameters to get_dsp()

[mikeoliphant]

If the prewarm() call in Reset() is just a bug, removing it resolves most of my issues.

It isn't clear, though, whether the rest of the internal code that calls Reset() is relying on that prewarm or not (ie: when slimmable size changes).

[rerdavies]

Global defaults for max buffer size is ok. But there are situations where you need to get a model in a specific size, without incurring an extra pre-warming penalty. E.g. loading a preset with 16 NAM instances in it, all of which have different sizes.

[sdatkinson]

To add a solution: to @mikeoliphant 's list: one could store additional state information in the model that helps infer whether prewarm is needed.

Off the top of my head, if:

• Reset() does not change max buffer size nor sample rate
• The model has not .process()ed since the last Reset() call

...then the current Reset() may be a no-op.

This would, if I'm not mistaken, get us only one prewarm, as expected. Feel free to point out if I missed something, but I believe that this could be implemented at the parent class level and would benefit basically every type of model :)

[sdatkinson]

...and it seems I forgot to mention this Issue in #267. D'oh.

[sdatkinson]

@mikeoliphant Regarding prewarming being part of Reset, good catch.

From my PoV today, I think I like prewarming during reset primarily because it makes this class looks less "special" compared to other audio effects.

But also, one could substantially improve the efficiency of pre-warming (albeit at the expense of memory) by including a place to hold the state after pre-warming (e.g.the ring buffer for a convolutional layer) and, instead of using .process() on subsequent calls, simply copy the results from the first call.

(In fact, this is actually part of the .nam file for the LSTM, though it doesn't work well enough to trust. IIRC I think the culprit is the tanh approximation.)

Nothing all that fancy; just more code complexity. If it's worth it, then I'm open to adding it.

[sdatkinson]

@rerdavies

And being able to call SetSlimmableSize() on a realtime thread simplifies a LOT of code for me.

I'd be interested to understand why this is the case. My thinking is influenced by iPlug2, where OnParamChange isn't RT. It seems reasonable to me to do it like that.

[mikeoliphant]

Not re-prewarming if process() hasn't been called doesn't quite work, I don't think.

Take the following sequence of events (which is actually what I currently do):

• get_dsp() is called, and calls prewarm()
• Reset() gets called to set the maximum buffer size - now prewarm() needs to be called again even though process() hasn't happened
• SetSlimmableSize() gets called - now prewarm() needs to be called (but only if it changes the internal model)

[sdatkinson]

@mikeoliphant that makes sense; thanks.

Let me think through your two suggestions:

allow setting global defaults for slim size and max buffer size

Makes sense. I bet I've got some defaults hard-coded in there somehow currently; making them compile-time definable may alleviate most of the issues without building the ability to change them at runtime, though the latter feels swell.

take slim size and max buffer size as parameters to get_dsp()

So long as they're optional, I'm open to this.

...I'll look into this today. I can vaguely imagine the changes that I'd be happy with--shouldn't be too hard.

[mikeoliphant]

Makes sense. I bet I've got some defaults hard-coded in there somehow currently; making them compile-time definable may alleviate most of the issues without building the ability to change them at runtime, though the latter feels swell.

They need to be runtime changeable. In a DAW plugin context, max buffer size comes at runtime. And the current "default" slimmable size (at least in my case) comes at runtime from the parameter value set by the user.