search

mikeoliphant / neural-amp-modeler-lv2

Neural Amp Modeler LV2 plugin implementation
GNU General Public License v3.0
229 stars 28 forks source link

Can not load .nam files from tonehunt #69

Closed mekanix closed 3 months ago

mekanix commented 4 months ago

I managed to compile and run this plugin on FreeBSD (thank you very much!) but I can't load any .nam files from tonehunt. When I look inside the .nam file in this repository, I can see "architecture": "LSTM" in it, but if I download from tonehunt, I see "architecture": "WaveNet". Two files look similar, but this is, I think, the biggest difference.

If there's some work needed, can you outline it and let me/us see if I/we can help? I mean, maybe I'm not the only one.

mikeoliphant commented 4 months ago

"LSTM" and "WaveNet" are just two types of network architectures. Both are supported. The LV2 plugin should be able to load any .nam file on ToneHunt.

mekanix commented 4 months ago

I'll dive into the code over the next days or weekend and see if I can narrow down where it breaks. Thank you!

mekanix commented 4 months ago

When I run this modified code it prints out

Loading model from Plugin::work /home/meka/repos/neural-amp-modeler-lv2/models/BossWN-feather.nam
Failed: Head not implemented!

Does it tell you something? I'll continue my search in the following days, but if you can help make the bug hunt faster, I'm all ears.

mikeoliphant commented 4 months ago

So, that's interesting. That error is supposed to get thrown if the WaveNet architecture has a head, which is not supported (and no NAM models should have one).

I was initially confused when I looked at the code that throws that error in the NAM Core, since it seems to have it's logic backward. Turns out it is two bugs that fix each other. It is checking whether the "head" element in the json is null by comparing it to null, when in fact it is a json object that represents null. But then it reverses the logic, which makes it work even though it is wrong:

https://github.com/sdatkinson/NeuralAmpModelerCore/blob/028e648772f9931b01b8286eed3a1cfe4d9f9bc2/NAM/get_dsp.cpp#L175

Anyway, for some reason when you are running it, it seems it is coming back actually null, which it shouldn't.

No idea why, though.

mikeoliphant commented 4 months ago

PR for fixing the aforementioned head check issue is here: https://github.com/sdatkinson/NeuralAmpModelerCore/pull/108

It wasn't causing any actual problems with existing .nam files, though, so it does not explain the problem you are seeing.

mekanix commented 4 months ago

I confirm this fixes my issue. Thank you for such a quick response! Do you want me to close this issue, or you'd like to close it once everything is merged?

mikeoliphant commented 3 months ago

I confirm this fixes my issue. Thank you for such a quick response!

That's good! I wish I understood why this was causing problems on your system (you're running FreeBSD, correct?) and not on Windows or Linux, though.

Do you want me to close this issue, or you'd like to close it once everything is merged?

Lets leave it open until it is resolved upstream and I get it merged back here.

mikeoliphant commented 3 months ago

This is now fixed upstream and merged.