Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can not load .nam files from tonehunt #69

Closed
mekanix opened this issue Jul 30, 2024 · 8 comments
Closed

Can not load .nam files from tonehunt #69

mekanix opened this issue Jul 30, 2024 · 8 comments

Comments

@mekanix
Copy link

mekanix commented Jul 30, 2024

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
Copy link
Owner

"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
Copy link
Author

mekanix commented Jul 31, 2024

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
Copy link
Author

mekanix commented Jul 31, 2024

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
Copy link
Owner

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
Copy link
Owner

PR for fixing the aforementioned head check issue is here: sdatkinson/NeuralAmpModelerCore#108

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

@mekanix
Copy link
Author

mekanix commented Aug 1, 2024

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
Copy link
Owner

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
Copy link
Owner

This is now fixed upstream and merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants