search

sdatkinson / neural-amp-modeler

Neural network emulator for guitar amplifiers.
MIT License
1.87k stars 150 forks source link

`core.py`: Stop deprecated use of `start` and `stop` when initializing data sets #424

Closed sdatkinson closed 6 months ago

sdatkinson commented 6 months ago

Use start_samples, stop_samples instead.

yonMaor commented 6 months ago

If possible I'd like to tackle this issue. Any other information I should know considering that this is my first time working on this project?

sdatkinson commented 6 months ago

@yonMaor great!

This one is pretty straightforward--just address the spots where code in core.py uses the deprecated arguments. I don't think you'll need to change anything outside of that file. Maybe in the corresponding test_core.py file if there's use of the deprecated argument there, but I don't remember off the top of my head.

sdatkinson commented 6 months ago

When you're done, make a PR and the automatic testing should make sure things are working as expected.

yonMaor commented 6 months ago

Which start/stop are you referring to? I'm seeing a lot of these exactly, but as arguments only as part of dictionary keys in validation/train kwargs.

sdatkinson commented 6 months ago

I'm not sure off the top of my head.

Basically, find anything in core.py that initializes a Dataset using the start and stop arguments, and have it instead use start_samples and stop_samples instead.

I'd recommend doing it by dropping a breakpoint on like line 300 then looking at the stack to see where it came from.

It'll probably often be via init_dataset.

Does this help?

yonMaor commented 6 months ago

Do you have any instructions on how to run the code? I tried running main, but it seems to require command-line arguments that I don't know where to get

yonMaor commented 6 months ago

Is it possible that these parameters are only explicitly written in the single_pair.json and two_pairs.json files? Digging around a bit that's where I've been able to find them

sdatkinson commented 6 months ago

E.g. https://github.com/sdatkinson/neural-amp-modeler/blob/825b810545516ce4556ba4e6cbde9f86641cb37b/nam/train/core.py#L916

Ctrl+F for "start".

You can see the entry point for the GUI trainer defined in setup.py: nam.train.gui:run. This is one of the two "simplified" trainers that uses train.core (the other is the Jupyter notebook that's meant to be used in Colab.)