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

Fix multiprocess in dev tracking #676

Merged
merged 4 commits into from
Mar 1, 2023

Conversation

EmmaRenauld
Copy link
Contributor

During multiprocessing Pool, the argument sent is a dict, but the multiprocessing sends it as *args, thus deconstructing the dict.
Modifying to (dict, ), so that instead it will diconstruct the tuple and leave the dict intact.

Not sure how we did not find this bug sooner... Sorry.

@arnaudbore arnaudbore self-requested a review February 17, 2023 20:12
@arnaudbore
Copy link
Contributor

Build passed ! Good Job 🍻 !

@CHrlS98
Copy link
Contributor

CHrlS98 commented Feb 20, 2023

What was the impact of this bug? Everything would crash? Cause I'm pretty sure I ran the dev tracking with multiprocessing successfully at some point...

@EmmaRenauld
Copy link
Contributor Author

Yes, everything crashed. And I don't understand, me too, I think it worked at some point. But yesterday, I ran it with this modified code and it worked.

@arnaudbore
Copy link
Contributor

Build passed ! Good Job 🍻 !

@arnaudbore arnaudbore requested a review from CHrlS98 March 1, 2023 18:04
Copy link
Contributor

@CHrlS98 CHrlS98 left a comment

Choose a reason for hiding this comment

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

Tested on python3.10; all works well 💯

@arnaudbore
Copy link
Contributor

Build passed ! Good Job 🍻 !

2 similar comments
@arnaudbore
Copy link
Contributor

Build passed ! Good Job 🍻 !

@arnaudbore
Copy link
Contributor

Build passed ! Good Job 🍻 !

@arnaudbore arnaudbore merged commit a14b2a6 into scilus:master Mar 1, 2023
@EmmaRenauld EmmaRenauld deleted the fix_multiprocess branch March 6, 2023 15:48
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