Dr-Noob / gpufetch

Simple yet fancy GPU architecture fetching tool
GNU General Public License v2.0
138 stars 15 forks source link

A few nitpicks (very minor changes) #16

Closed CodePurble closed 2 years ago

CodePurble commented 2 years ago

These are a few tiny changes that I am "reporting" here because of the no-PR policy. Takes seconds to implement.

I found out that the CUDA_DRIVER_START_WARNING message does not end with a newline, causing any subsequent error messages to start on the same line as the warning. Adding a newline would just even things out.

Also, adding build/ to the gitignore is another thing you can consider.

Thanks and regards

Dr-Noob commented 2 years ago

Hi, thanks for the suggestions.

Unfortunately, I don't have too much time to dedicate to this project so I'm aware that there are some minor issues that need some care. I just took your suggestion for the .gitignore but I'm doubtful about the other thing.

Looking at the code, it looks like the only message that can arise after the CUDA_DRIVER_START_WARNING message is the call to cudaGetDeviceCount. Or does it show any other message to you that I'm not considering? The thing about adding a newline to the message is that it could not be removed by \r because the message would be in another line, so I would need to add ANSI escape characters to go back to the previous line, which is something I would prefer avoiding.

CodePurble commented 2 years ago

image

Hi @Dr-Noob,

About the newline, what happens is what is shown in the screenshot above. The [ERROR]: message starts immediately after the CUDA_DRIVER_START_WARNING message, which is a little disorienting. I don't think there are any other cases where this can occur (I use a laptop with optimus-manager, so CUDA isn't loaded if I am in integrated graphics mode, causing the warning).

Dr-Noob commented 2 years ago

Ah, I see. Well, that's a small detail. But these details matter! I just pushed a fix for that case. Can you confirm that now it works as expected?

CodePurble commented 2 years ago

It works, confirmed. Thanks!