LOG.info not printing at all? (v1.3.2)

[Lordmau5]

Describe the bug
LOG.info calls aren't printing anything to the console
Example LOG.info(f"====> Epoch: {epoch}, cost {durtaion} s")

To Reproduce
Running svc train with the specific model path should do the trick

Additional context
I can see LOG.warning calls, especially since I've added those as well, and I couldn't spot anything in regards to the log level being set to only warnings instead of info. I remember seeing info logs in the Google colab I've used though (Might've been with the code from the main repository though)

[34j]

Warning is the (Python) default level. can you describe more about specific path?

[Lordmau5]

According to this line it should be set to INFO though, right?
https://github.com/34j/so-vits-svc-fork/blob/main/src/so_vits_svc_fork/__main__.py#L35

can you describe more about specific path?

I have a batch script that I can double-click which has the following code:
svc train --model-path logs/44k

[34j]

I cannot reproduce this issue. Can you please explain more about this?

The getLogger change is not necessary and I am sorry but I don't understand what you are talking about.

[Lordmau5]

Example log file of running with the up-to-date code on your repository:
so_vits_svc_fork.log

Since it didn't log anything besides the version, here's a screenshot of what the terminal showed:

---

Example log file with the changes I made:
so_vits_svc_fork.log

Additionally, a screenshot, too:

---

So yes, the getLogger change is necessary.

Edit: I should additionally mention that I'm running on Windows 11.
Maybe it works correctly on Linux? Or even Windows 10? Not sure

[34j]

There must be some other solution, though I can't think of any right now...

[Lordmau5]

There must be some other solution, though I can't think of any right now...

What do you mean "other solution"? The solution that I provided in the pull request works.
I didn't modify any internal logging modules or their methods, such as getLogger like you said.
All I did was do a wrapper class that is properly instanciated when it's being used in files.

The current code you have is only getting a standard logger without any configuration, which means it had it's defaults, besides in the main file.

However, that means that it also doesn't have proper formatting (or even the RichHandler)

This addresses that problem.

Additionally, I feel adding a launch argument to set the actual log level might be good to have, too
Such as --log=INFO or similar

[34j]

python -c "import so_vits_svc_fork.train"
python -c "import so_vits_svc_fork.__main__"

> python -c "import so_vits_svc_fork.train"
> python -c "import so_vits_svc_fork.__main__"
[20:17:49] INFO     [20:17:49] Version: 1.3.4           

I do not want to or will not merge code that does not meet these two requirements.

[34j]

Does your code meet these?

[Lordmau5]

Yep, just ran these

I fully understand that you don't want to merge code that doesn't meet specific requirements.
If there's anything else you want me to test, I'd be more than happy to 😁

I should mention I'm not able to edit anything until tomorrow because I'll be gone for today

[34j]

Sorry, I meant this.

python -c "import so_vits_svc_fork.__main__;from logging import getLogger;getLogger().info('hello')"
python -c "import so_vits_svc_fork.train;from logging import getLogger;getLogger().info('hello')"

Only the first code should produce output. In other words, it is not a good idea to change the Handler etc when using it as a module.
Though I can certainly understand the opinion that no one may use this package as a module...

[34j]

It is enough to start up tensorboard for me and I don't want to touch this horrible problem for a while.

[Lordmau5]

Yeah I personally don't use Tensorboard but I'd like some extra information in the console window haha

Those do produce the wrong results as you mentioned.

Would it in that case be better to instead have a launch parameter as I proposed, such as --log=LOG_LEVEL, which then allows finer control?

__main__ would still use the default that we have, but we could supply the log level additionally through this for the other places where it uses the logger

[34j]

from .__main__ import init_logger
init_logger()

Actually, the behavior you are describing can be easily implemented by simply adding these two lines to __init__.py in the first place.
Why BlueAmulet gives me a low rating mark is a mystery to me. I rather like to give it to him.

[Lordmau5]

Oh wait, there's an init.py file? I didn't even notice that haha

Additionally though, the LOG.info(f"Version: {__version__}") and LOG.debug("Test mode is on.") need to be moved out of the initLogger method

Otherwise we get this every single time something happens. (Notice the multiple logs of Version)

I tried removing the call to init_logger() from the __main__.py file and instead put it into __init__.py, but that doesn't seem to retain the logger initialization it seems... (meaning that LOGGER_INIT is being set to False again for whatever reason)

[34j]

It is a multi-processing problem that requires some refactoring to solve, but there is absolutely no need to make the major changes that BlueAmlet mentions.

[Lordmau5]

It is a multi-processing problem that requires some refactoring to solve, but there is absolutely no need to make the major changes that BlueAmlet mentions.

Understood, in that case please keep the pull request closed and I can look into making a new one that would fix those issues soon

I guess for now I'll just keep init_logger() inside the __init__.py file and comment out the 2 LOG. calls inside __main__.py :D

[34j]

Glad to finally get the word out. Now I can sleep peacefully.

[BlueAmulet]

Why BlueAmulet gives me a low rating mark is a mystery to me. I rather like to give it to him.

I gave you a Confused Face emoji, because I was confused about what you said before.

Yes you are correct, there was no need to make the major changes that I proposed, simply calling init_logger from __init__.py would have been enough to get the behavior that Lordmau5 and I desired.

[34j]

I see, I misunderstood. I am very sorry. I withdraw my statement.

The repository had grown too large and I had lost my mental capacity (excuse).

[Lordmau5]

@BlueAmulet Could you test with the latest code as well and see if it's a similar result to what you expected, too?

[Lordmau5]

Gonna assume this is working properly for others, too - I've not noticed any more issues myself