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

TrackStack - multi cores support #224

Merged
merged 9 commits into from
Nov 1, 2023

Conversation

aitov
Copy link
Contributor

@aitov aitov commented Oct 12, 2023

Current version of TrackStack supports only one core for stacking and it takes a lot of time for big amount of meteors (i.e. in meteor shower maximum it could be up to 200-300)
This version is attempt to improve performance as modern CPUs have 4 and more cores.

I made tests for stacking of 30 meteors and got next results on my test environments:

Mac M2 Pro (10 cores)
single core time : 1 min 20 sec
all cores time : 21 sec

Raspberry Pi 4 (4 cores) (my friend still using Pi for stacks )
single core time : 18 min 23 sec
all cores time : 5 min 53 sec

Any suggestions - welcome

Copy link
Contributor

@markmac99 markmac99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you put the TQDM changes back? This was added to make display more user-friendly progress by presenting a thermometer type progress bar rather than a scrolling list. I hope it can still be used in the multithreaded case?

Also typo at line 339, should be "Stacking".

@aitov
Copy link
Contributor Author

aitov commented Oct 25, 2023

Hi, TQDM could not be used in multi core as it wrapper on list (or I need suggestion how to integrate it). I made inline thermometer which looks pretty same but without remaining time and scrolls only when QueuedPool prints current jobs state (probably need add option to suppress prints)

Also I thought to create new file with name like StackTrack2 to leave current as is, I can do if it is preferable option. But it means someone will use old and wait ))
Yesterday I created track stack for 280 full hd meteors for only 4min

printProgress method

@markmac99
Copy link
Contributor

ok, ref tqdm, that makes sense. I did wonder if that might be a problem, and anyway it hopefully removes a dependency. I'll pull this branch to one of my systems and run it for a couple of days if thats ok?

@aitov
Copy link
Contributor Author

aitov commented Oct 25, 2023

Sure, I think we need more testing but can't find volunteers with different envs, I'm testing on everyday basis but probably exists other issues which cannot be reproduced on Mac or Raspberry Pi.

@markmac99
Copy link
Contributor

I think the progress bar code is not valid on a Pi3 because the end= kwarg is not supported in python 2.7.
print("\rStacking : {:02.0f}%|{}{}| {}/{} ".format(percent, "█" * progress, " " * (progress_bar_len - progress), current, total), end="")
^
SyntaxError: invalid syntax

You might need to put some conditionality round this
if sys.version_info[0] < 3:
use python-2 compatible code
else:
use python-3 code

@dvida
Copy link
Contributor

dvida commented Oct 25, 2023

You can probably just put "from future import print_function" at the top and that should solve it.

@markmac99
Copy link
Contributor

Also i have a problem with the unicode character in that print() statement which generates an error on my test pi4 as its not a valid character in ISO-8859-1.
This turns out to be because my test pi4 does not have en_US.UTF-8 locale enabled. I enabled it and then everything worked, but i don't think we can guarantee that en_US.UTF-8 will be enabled on all target machines and so the unicode character might be a problem.
You might need to test whether the character is available (put a try/except round the code?), and then use a different character if the block character isn't available.

@markmac99
Copy link
Contributor

markmac99 commented Oct 25, 2023

@dvida yes, that works though it should be

from __future__ import print_function 

(I think github removed the underscores from your comment!)

@markmac99
Copy link
Contributor

markmac99 commented Oct 25, 2023

I tested with a set of 188 meteors.
Previously, it took 140 minutes to stack these data on my "production" Pi4 with 2GB memory.

After deploying this update, the same stack took:
8 minutes on a quad-core i5-6500 with 48GB memory running Windows 10.
45 minutes on a quad-core Pi4 with 1GB memory, running Buster.
75 minutes on a quad-core Pi3 with 1GB memory. running Jessie with python 2.7, after Denis' suggestion to fix the print function, however drawConstellations crashed preventing the file being saved,
File "Utils/DrawConstellations.py", line 37, in drawConstellations
cv2.line(img, (round(from_x[i]), round(from_y[i])), (round(to_x[i]), round(to_y[i])), color_bgr, 1)

On both Pis, the "Stacking" percentage progress bar only appeared occasionally, mostly being skipped or perhaps overwirtten by messages such as the below. I don't think this is a big issue, as stacking did complete successfully albeit with the above problem with drawConstellations.
' -----
Cores in use: 4
Active worker threads: 4
Idle worker threads: 0
Total jobs: 182
Finished jobs: 72
' -----
Cores in use: 4
Active worker threads: 4
Idle worker threads: 0
Total jobs: 182
Finished jobs: 76
' -----

@aitov
Copy link
Contributor Author

aitov commented Oct 26, 2023

Hi, thank you for your help!
From side I'm going to :

  • create venv and test on Python 2.7 and check drawConstellations issue
  • replace "█" (copied from TQDM) symbol by "#" to support also non unicode systems (or check if UTF-8 not supported)
  • add flag to QueuedPool to control prints of state which will be enabled by default but disabled for TrackStack (is it ok?)

@aitov
Copy link
Contributor Author

aitov commented Oct 26, 2023

Also i have a problem with the unicode character in that print() statement which generates an error on my test pi4 as its not a valid character in ISO-8859-1. This turns out to be because my test pi4 does not have en_US.UTF-8 locale enabled. I enabled it and then everything worked, but i don't think we can guarantee that en_US.UTF-8 will be enabled on all target machines and so the unicode character might be a problem. You might need to test whether the character is available (put a try/except round the code?), and then use a different character if the block character isn't available.

@markmac99 could you please specify what error message you got?
during testing on python 2.7 I got next 2 errors:

  • Compile error: SyntaxError: Non-ASCII character '\xe2' in file
  • Runtime error: UnicodeEncodeError: 'ascii' codec can't encode characters in position 0-1: ordinal not in range(128)
  • Other message?

@aitov
Copy link
Contributor Author

aitov commented Oct 27, 2023

create venv and test on Python 2.7 and check drawConstellations issue

This issue also present in current version - fixed by type cast

replace "█" (copied from TQDM) symbol by "#" to support also non unicode systems (or check if UTF-8 not supported)

I tested with code bellow and all issues which I had - resolved, need test with locale ISO-8859-1 (not sure how to set it), I'm expecting no errors but it could print something wrong

symbol = u'\u2588'
try:
     symbol = str(symbol)
except UnicodeEncodeError:
     symbol = symbol.encode('utf-8')

add flag to QueuedPool to control prints of state which will be enabled by default but disabled for TrackStack (is it ok?)

added flag and tested on Raspberry Pi - looks good

@markmac99
Copy link
Contributor

Thanks, I'll pull this and run some tests later today on Windows, Pi4 and Pi3. Will also try on Ubuntu if i get time.

@markmac99
Copy link
Contributor

unfortunately, still getting a unicode error on my Pi4
Stacking : 05%| | 9/182 Traceback (most recent call last):
File "/home/pi/source/RMS/RMS/QueuedPool.py", line 356, in _workerFunc
result = func(*all_args, **self.func_kwargs)
File "/home/pi/source/RMS/Utils/TrackStack.py", line 430, in stackFrame
printProgress(finished_count.value, num_ffs)
File "/home/pi/source/RMS/Utils/TrackStack.py", line 454, in printProgress
print("\rStacking : {:02.0f}%|{}{}| {}/{} ".format(percent, symbol * progress, " " * (progress_bar_len - progress), current, total), end="")
UnicodeEncodeError: 'latin-1' codec can't encode character '\u2588' in position 16: ordinal not in range(256)

I'll do some testing to see if i can find the workaround.

@markmac99
Copy link
Contributor

Unfortunately, if the OS doesn't understand UTF-8, then the symbols with a value > 255 can't be printed whether encoded or not and str() of the unicode character just converts it into a unicode string I think.
You could use a try/except round the print statement, and use a different symbol entirely if it fails?
try:
symbol='█'
print(...)
except Exception:
symbol = 'X'
print(...)

ISO-8859-1 is another name for 256-bit ASCII.

@aitov
Copy link
Contributor Author

aitov commented Oct 27, 2023

symbol='█'

it produced error on my env: SyntaxError: Non-ASCII character '\xe2' in file
seems like it could be fixed by adding in file:

# -*- coding: utf-8 -*-

but if no utf-8 support not sure how it could be handled

Is your snipped above working for ISO-8859-1 ?

@dvida
Copy link
Contributor

dvida commented Oct 27, 2023

I might be missing something, but why not have a progress bar that looks something like:
[XXXXXX-----------]

That should be fully cross-platform and only uses ASCII characters. It's not as pretty, but it does the job.

@aitov
Copy link
Contributor Author

aitov commented Oct 27, 2023

That should be fully cross-platform and only uses ASCII characters. It's not as pretty, but it does the job.

Agree, initial try was copy progress bar from TQDM lib.
@markmac99 I committed one more version - could you please try one more time?

if will not help - let's replace with "#" symbol.

@markmac99
Copy link
Contributor

markmac99 commented Oct 27, 2023

No luck I'm afraid. On the Pi4 it still fails with
File "/home/pi/source/RMS/Utils/TrackStack.py", line 450, in printProgress
UnicodeEncodeError: 'latin-1' codec can't encode character '\u2588' in position 16: ordinal not in range(256)

The problem is, neither of these tests fail

  symbol = str(symbol)
  "{}".format(symbol)

because in Python3, I believe all strings are unicode strings (this is different to python2 where strings are single-byte-character arrays). And so the Except... isn't triggered.

The only safe test is whether the print() statement fails, i think. But i agree with Denis comment, just use an eASCII character!

I did get it to work ok with this:

    try:
        symbol = u'\u2588'
        print("\rStacking : {:02.0f}%|{}{}| {}/{} ".format(percent, symbol * progress, " " * (progress_bar_len - progress), current, total), end="")
    except Exception:
        symbol = 'X'
        print("\rStacking : {:02.0f}%|{}{}| {}/{} ".format(percent, symbol * progress, " " * (progress_bar_len - progress), current, total), end="")
Using 4 cores
Stacking : 100%|XXXXXXXXXXXXXXXXXXXX| 182/182
All workers are idle!

@aitov
Copy link
Contributor Author

aitov commented Oct 27, 2023

Replaced with "#", for me it looks better than "X"

Stacking : 100%|####################| 30/30 
All workers are idle!

@dvida dvida merged commit c5ed641 into CroatianMeteorNetwork:master Nov 1, 2023
@aitov aitov deleted the track-stack-improvements branch November 1, 2023 18:17
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

Successfully merging this pull request may close these issues.

3 participants