bpennypacker / phad

Pi-Hole Alternate Display
GNU General Public License v3.0
75 stars 8 forks source link

ZeroDivisionError: float division by zero at line 692 #19

Closed 33b5e5 closed 3 years ago

33b5e5 commented 3 years ago

Not quite sure what's causing this yet, but on first run of phad on a Raspberry Pi 4 Model B Rev 1.2 I'm getting this traceback:

$ ./phad
Warning: Moudle evdev not found. Touch screen support is disabled.
Traceback (most recent call last):
  File "./phad", line 1174, in <module>
    data = fetch_data(config)
  File "./phad", line 774, in fetch_data
    data['host'] = get_host_data(config)
  File "./phad", line 692, in get_host_data
    data['cpuload'] = data['loadavg'][0] / data['numprocs'] * 100
ZeroDivisionError: float division by zero

/proc/cpuinfo on this machine:

$ cat /proc/cpuinfo
processor       : 0
BogoMIPS        : 108.00
Features        : fp asimd evtstrm crc32 cpuid
CPU implementer : 0x41
CPU architecture: 8
CPU variant     : 0x0
CPU part        : 0xd08
CPU revision    : 3

processor       : 1
BogoMIPS        : 108.00
Features        : fp asimd evtstrm crc32 cpuid
CPU implementer : 0x41
CPU architecture: 8
CPU variant     : 0x0
CPU part        : 0xd08
CPU revision    : 3

processor       : 2
BogoMIPS        : 108.00
Features        : fp asimd evtstrm crc32 cpuid
CPU implementer : 0x41
CPU architecture: 8
CPU variant     : 0x0
CPU part        : 0xd08
CPU revision    : 3

processor       : 3
BogoMIPS        : 108.00
Features        : fp asimd evtstrm crc32 cpuid
CPU implementer : 0x41
CPU architecture: 8
CPU variant     : 0x0
CPU part        : 0xd08
CPU revision    : 3

Hardware        : BCM2835
Revision        : c03112
Serial          : 10000000aeee4cc3
Model           : Raspberry Pi 4 Model B Rev 1.2
33b5e5 commented 3 years ago

https://github.com/bpennypacker/phad/blob/d64d5d07b47b23adb1d4d3d40bd1ae0fd4272cd5/phad#L686-L692

The problem starts on line 686, because on Ubuntu 20.04 for the Pi, /proc/cpuinfo doesn't use the string "model name", but rather "Model" instead. So data['numprocs'] = 0, then you get a divide by zero on line 692.

Could just grep for "model", case insensitive, but I think that would need more testing on other distros. Or maybe you could just search for "model name" OR "Model" for now. I'm surprised there is variance in the naming in /proc/cpuinfo.

I'm not sure I understand the logic in the else clause though. Why set data['numprocs'] = 0?

33b5e5 commented 3 years ago

After a little more digging it seems "model name" is the most common way to label cores in /proc/cpuinfo. I don't know why Ubuntu 20.04 for aarch64/arm64 omits this. "Model" is there, but only per physical cpu.

Could use this instead: grep -c "^processor" /proc/cpuinfo. I tested on a handful of machines including other architectures, and it seems to correctly count the cores.

This might be more robust but would require importing another module:

import multiprocessing
multiprocessing.cpu_count()

I forgot to say thank you for the project. Thank you!

bpennypacker commented 3 years ago

Thanks for digging into this. I actually hadn't bothered to test this on Ubuntu so I probably should do a full test of that. All my pi's are current model 3, so this may be a good time to go and invest in a RPi 4...

I'm not sure I understand the logic in the else clause though. Why set data['numprocs'] = 0?

That's just in the event grep fails, which it hopefully never would. The run function will timeout if a subprocess doesn't return in reasonable time (default of 30 seconds), so if the grep call hangs or otherwise returns an error it just defaults to setting the value to 0.

And yes, it should be easy enough to change that grep to something more like grep -ic "model" /proc/cpuinfo to cover that subtle difference between Ubuntu & raspbian.

I'm going to go order an RPi 4, and I'll give Ubuntu a try when I get it.

combro2k commented 3 years ago

I have the same message in Debian Buster 64 bits :-)

Traceback (most recent call last): File "./phad", line 1174, in data = fetch_data(config) File "./phad", line 774, in fetch_data data['host'] = get_host_data(config) File "./phad", line 692, in get_host_data data['cpuload'] = data['loadavg'][0] / data['numprocs'] * 100 ZeroDivisionError: float division by zero

33b5e5 commented 3 years ago

And yes, it should be easy enough to change that grep to something more like grep -ic "model" /proc/cpuinfo to cover that subtle difference between Ubuntu & raspbian.

FYI, I don't think that will work. On Ubuntu, "Model" represents a physical processor, but "model" shows up for each core on other common distros. I also found other examples where both "model" and "model name" appear separately, like:

model           : 45
model name      : Intel(R) Xeon(R) CPU E5-2660 0 @ 2.20GHz

This is why I suggested grep -c "^processor" /proc/cpuinfo instead.

That's just in the event grep fails, which it hopefully never would. The run function will timeout if a subprocess doesn't return in reasonable time (default of 30 seconds), so if the grep call hangs or otherwise returns an error it just defaults to setting the value to 0.

But won't this always result in a divide by zero and traceback? Might it make more sense to default to data['numprocs'] = 1 so it doesn't crash?

combro2k commented 3 years ago

Perhaps an even more easier approach: nproc --all

On most systems its available at coreutils

bpennypacker commented 3 years ago

This should be fixed now. I'd suggest just downloading the phad script itself and not running the install script again.

33b5e5 commented 3 years ago

Looks good to me on the Ubuntu Pi 4 B's. Thank you!

bpennypacker commented 3 years ago

Great, thanks for the quick confirmation!