GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-17560: Too small type for struct.pack/unpack in mutliprocessing.Connection by ahcub · Pull Request #10305 · python/cpython

Conversation

@ahcub

@ahcub ahcub commented Nov 3, 2018 •
edited by serhiy-storchaka
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

https://bugs.python.org/issue17560

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Light--, espdev, prateek-77, conradogarciaberrotaran, PGijsbers, johntiger1, JZL, serjtroshin, ye-sun-econ, and eliasyin reacted with thumbs up emoji Light-- reacted with laugh emoji Light-- and conradogarciaberrotaran reacted with hooray emoji Light-- reacted with heart emoji Light-- reacted with rocket emoji
All reactions
  • 👍 10 reactions
  • 😄 1 reaction
  • 🎉 2 reactions
  • ❤️ 1 reaction
  • 🚀 1 reaction
ahcub added 13 commits September 1, 2018 19:50
@ahcub
Fix for invalid assert on big output of multiprocessing.Process
19481ec
@ahcub
add the news
95bfc95
@ahcub
remove the changes from generated file
c622aae
@ahcub
Remove changes in the generated part and made changes in the clinic i…
5c8ffcb
…nput instead
@ahcub
!U add test for big output multiprocessing check
31c90f9
@ahcub
!U commit changes in the generated files
a1b8f7b
@ahcub
!U remove test for big output because it is only for manual use
26c88c4
@ahcub
!U remove the manual change
4e5026f
@ahcub
!U fix the generated code parts
0d31498
@ahcub
!U changed the News entry
92f8ece
@ahcub
Merge pull request #1 from python/master
c54bb14
!I cpython/master -> ahcub/cpython/master
@ahcub
Merge pull request #2 from python/master
c11a1d7
!I cpython/master -> ahcub/master
@ahcub
[35152] too small type for struct.pack/unpack in mutliprocessing.Conn…
6dd800b
…ection
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Nov 3, 2018
@bedevere-bot bedevere-bot added the awaiting review label Nov 3, 2018
@ahcub
!U add news entry
5744461
@ahcub

ahcub commented Nov 3, 2018

Copy link
Copy Markdown
Contributor Author

side note: currently on Windows we use DWORD for multiprocessing.Process output size which is, as far as I understand, an equivalent of !Q

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ahcub added 2 commits November 3, 2018 11:03
@ahcub
!U change the news entry
d437efa
@ahcub
!U change the news entry
e4551dd
pitrou
pitrou requested changes Nov 3, 2018
View reviewed changes

@pitrou pitrou left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Hi and thanks for trying to solve this. You're on the right track here, but we should maintain compatibility with older versions (one should be able to set up a Listener with a Python version and a Client with another Python version, though that's probably uncommon). One way to do that is as outlined in my message below (simply put, use a special value of the 32-bit size field to introduce a larger 64-bit size field):
https://bugs.python.org/issue17560#msg185345

Also, please change the issue number reference to the non-closed issue :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-bot bedevere-bot added awaiting changes and removed awaiting review labels Nov 3, 2018
@bedevere-bot

bedevere-bot commented Nov 3, 2018

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pitrou

pitrou commented Nov 3, 2018

Copy link
Copy Markdown
Member

side note: currently on Windows we use DWORD for multiprocessing.Process output size which is, as far as I understand, an equivalent of !Q

On Windows it's PipeConnection that's used by default. Unfortunately many Windows APIs (such as ReadFile) are not 64-bit clean internally. We would have to introduce message chunking. But that can be done in a later PR if there's a Windows developer that's interested in that.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ahcub

ahcub commented Nov 3, 2018

Copy link
Copy Markdown
Contributor Author

thanks for the note and description
I understand what you mean, but I'm not sure how this use case is possible. Can you show an example of how to setup different versions for multiprocessing Client and Listener?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pitrou

pitrou commented Nov 3, 2018

Copy link
Copy Markdown
Member

In one terminal:

Python 3.7.1 (default, Oct 23 2018, 19:19:42) 
[GCC 7.3.0] :: Anaconda, Inc. on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from multiprocessing.connection import Listener    
>>> listener = Listener(address='127.0.0.1:3456')
>>> conn = listener.accept()  # will wait for inbound connection
>>> conn.send(b'foo')

In the other:

Python 3.6.5 (default, Apr  1 2018, 05:46:30) 
[GCC 7.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from multiprocessing.connection import Client
>>> conn = Client(address='127.0.0.1:3456')
>>> conn.recv()   # will wait for incoming message
b'foo'
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ahcub

ahcub commented Nov 3, 2018

Copy link
Copy Markdown
Contributor Author

thanks, I will work on that

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@serhiy-storchaka serhiy-storchaka changed the title bpo-35152 too small type for struct.pack/unpack in mutliprocessing.Connection bpo-17560: Too small type for struct.pack/unpack in mutliprocessing.Connection Nov 5, 2018
@ahcub

ahcub commented Nov 6, 2018 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

ok, so I gave -1 a try and it seems like it will not support the diff versions case as well.
the result is basically pretty much the same as if we connect a version with !Q change and the old one - connection hangs.
I was thinking about it, and I'm not sure if there is a solution that will support the different versions connection at all.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ahcub

ahcub commented Nov 6, 2018

Copy link
Copy Markdown
Contributor Author

ok, I guess you meant that with -1 approach will support the backward compatibility for cases when users don't hit the memory threshold of over 2GB

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot added the awaiting change review label Nov 6, 2018
@bedevere-bot

bedevere-bot commented Nov 6, 2018

Copy link
Copy Markdown

Thanks for making the requested changes!

@pitrou: please review the changes made to this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

pitrou
pitrou approved these changes Nov 6, 2018
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting change review labels Nov 6, 2018
@pitrou

pitrou commented Nov 6, 2018

Copy link
Copy Markdown
Member

Thank you @ahcub! I will merge this now.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pitrou
pitrou merged commit bccacd1 into python:master Nov 6, 2018
@bedevere-bot bedevere-bot removed the awaiting merge label Nov 6, 2018
@ahcub

ahcub commented Nov 6, 2018

Copy link
Copy Markdown
Contributor Author

thanks!

summerela reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sergpolly sergpolly mentioned this pull request Nov 24, 2018
pickles break while "multiprocess"-ing dots open2c/cooltools#50
Closed
@londumas londumas mentioned this pull request Jan 10, 2019
Rectangle distortion matrix igmhub/picca#528
Merged
@Hoohm Hoohm mentioned this pull request Jul 22, 2019
Two types of errors, that seem related to number of threads Hoohm/CITE-seq-Count#69
Closed
@ardunn ardunn mentioned this pull request Oct 10, 2019
Multiprocessing error when featurizing many band structure objects hackingmaterials/matminer#417
Closed
@simonvh simonvh mentioned this pull request Nov 8, 2019
gimme maelstrom crashes vanheeringen-lab/gimmemotifs#81
Closed
@calum-chamberlain calum-chamberlain mentioned this pull request Nov 18, 2019
catalog_to_dd.write_correlations struct.error eqcorrscan/EQcorrscan#360
Closed
@gmrukwa gmrukwa mentioned this pull request Dec 9, 2019
Fixup: multiprocessing data passing gmrukwa/divik#16
Merged
@prateek-77 prateek-77 mentioned this pull request Mar 27, 2020
struct.error: 'i' format requires -2147483648 <= number <= 2147483647 open-mmlab/mmdetection#2044
Closed
@yangyxt

yangyxt commented May 24, 2020

Copy link
Copy Markdown

I was stuck with python version < 3.8 since a tool I have to use is not compatible with higher version. So I tried to make the changes to the connection.py as instructed above. However, I found the overhead time is ridiculously big.

I'm working with pandas DataFrame. The table was so big so I have to read it in chunks. I did not use pd.read_csv and chunksize for some reason. Instead, I used a custom generator based on built-in read. Then I pass every chunk of df I read to a function to do some filtration of rows based on values of certain columns using pool.apply_async() wrapped in lambda and built-in map(). The reason I did not use map_async() is that I need to pass multiple parameters to the function.

Then I used some timestamp in the logging and found out the time between yielding a chunk dataframe and start executing the function is near 1 min. This is totally unacceptable. I have 10 million rows in the table in total, and every time my generator only yields 5000 lines. I tried the same script on a table with only 100 thousand rows and yield 5000 lines each time. And the time between yielding a chunk df and start executing the function is near 10 ms. Why the total size of the file I'm about to read with generator matters that much in this case. Why passing the 5000 lines table(only 15 columns) to a function takes that much time?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ahcub

ahcub commented May 24, 2020

Copy link
Copy Markdown
Contributor Author

Hi @yangyxt, I would love to help, but would probably need to see the code to advice anything meaningful.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@yangyxt

yangyxt commented May 24, 2020

Copy link
Copy Markdown

Hi @yangyxt, I would love to help, but would probably need to see the code to advice anything meaningful.
Thanks. Glad to paste code here. This is how I implement mp.pool:

pool = mp.Pool(processes=(int(estimate_cpus()))) logger.warn("\n\nThe number of usable CPUs is: " + str(int(estimate_cpus()))) # Read the large bam using self-defined iterator based on pysam, yielding chunk dfs iterator = read_bam_qname_groups(bam, chunksize=chunksize, headers=headers, sep='\t') output = map(lambda chunk: pool.apply_async(func_return_unit_df, args=(chunk, *func_args)), iterator) results = map(lambda r: r.get(), output)

Notice read_bam_qname_groups() is a self-defined generator to read big tables in chunks, yielding 5000 lines at each time.

And func_return_unit_df is a placeholder for functions used to process that 5000 line chunk pandas dataframe. Since I need to implement *args here so I have to use built-in map() and lambda and pool.apply_async() instead of just using pool.map_async().

This is how I process after getting the "results" iterator.
chunk_df = next(results) logger.info("From {}, the returned result df's shape is ".format(func_return_unit_df.__name__) + str(chunk_df.shape) + "\n") chunk_df.to_csv(output_path, sep='\t', index=False, header=False, mode='a', encoding='utf-8')

Therefore, the workflow should be reading a big file into chunks, pass each chunk to func_return_unit_df, then output the processed result into an output file in appending mode. Doing this will prevent the script eating too much memory.

Furthermore, I used a logging command in self-defined "read_bam_qname_groups":
logger.info("We check the next the row and its qname is different from returned table's last row, the yielding table shape is: " + str(chunk_df.shape) + str(chunk_df.head())) yield chunk_df
Before yielding chunk_df, I output a logging info with a timestamp.

Then I put a logging command at the begining of func_return_unit_df.
def fetch_multi_with_boolarray(chunk, uqname): logger.info("The input chunk shape is: " + str(chunk.shape))
Once the function start executing, the logging info is output with a timestamp.

Here is the key part, I test the code in a table with 10 million rows
and I noticed that the interval between two timestamps is near 1 min. And I test the code in a table with 100 thousand rows and the interval between two timestamps is around 20 ms.
I don't know why this happens. This is really important to me. Pls let me know your opinions. Thanks!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ahcub

ahcub commented May 24, 2020

Copy link
Copy Markdown
Contributor Author

the first thing that I would recommend is to increase the chunk size to something like 100k lines per chunk, since you are most likely will waste more time on the data transfer than the parsing of that chunk.

but even with 5k rows per chunk, I'm not sure what is going on there that makes it run for a minute.
I made a few tests on my local machine with 10million rows and 15 columns and with multiproc it finishes in 6 seconds if I chunk data by 100k rows and 9 seconds if I chunk by 5k rows.

another thing is that if you have files only 10million rows long and 15 columns wide you can simply read the file as is and don't do chunking at all. on my machine it finishes in 11seconds which doesn't seem long.

I can share the code I used for tests, so maybe it can be helpful for you

def process(data):
    df = pd.read_csv(data)
    return (df.shape, df.sum().sum())

chunk_size = 10**5
if __name__ == '__main__':
    with open('file.csv', 'rb') as file:
        content_lines = file.read().splitlines(keepends=True)
        print(len(content_lines))
        with Pool(4) as pool:
            start = datetime.now()
            tasks = []
            for i in range(0, len(content_lines), chunk_size):
                print('starting task', i)
                tasks.append(pool.apply_async(process, (BytesIO(b''.join(content_lines[i:i+chunk_size])),)))
            for task in tasks:
                print(task.get())
            print(datetime.now() - start)

not sure if I answered your question or not, but I hope this is helpful.
let me know if you have other questions.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@yangyxt

yangyxt commented May 25, 2020

Copy link
Copy Markdown

the first thing that I would recommend is to increase the chunk size to something like 100k lines per chunk, since you are most likely will waste more time on the data transfer than the parsing of that chunk.

but even with 5k rows per chunk, I'm not sure what is going on there that makes it run for a minute.
I made a few tests on my local machine with 10million rows and 15 columns and with multiproc it finishes in 6 seconds if I chunk data by 100k rows and 9 seconds if I chunk by 5k rows.

another thing is that if you have files only 10million rows long and 15 columns wide you can simply read the file as is and don't do chunking at all. on my machine it finishes in 11seconds which doesn't seem long.

I can share the code I used for tests, so maybe it can be helpful for you

def process(data):
    df = pd.read_csv(data)
    return (df.shape, df.sum().sum())

chunk_size = 10**5
if __name__ == '__main__':
    with open('file.csv', 'rb') as file:
        content_lines = file.read().splitlines(keepends=True)
        print(len(content_lines))
        with Pool(4) as pool:
            start = datetime.now()
            tasks = []
            for i in range(0, len(content_lines), chunk_size):
                print('starting task', i)
                tasks.append(pool.apply_async(process, (BytesIO(b''.join(content_lines[i:i+chunk_size])),)))
            for task in tasks:
                print(task.get())
            print(datetime.now() - start)

not sure if I answered your question or not, but I hope this is helpful.
let me know if you have other questions.

Dear ahcub,
Thank you so much for spending that time doing tests for my issue!
To clarify, I not only need to use this script to process tables with 10 million rows but also need it to process 100 million rows. So I have to read the tables in chunks.

I also had no clue about this issue. I found an article here https://thelaziestprogrammer.com/python/a-multiprocessing-pool-pickle but I don't have enough background knowledge to understand it. Does this remind you of anything?

Another thing confuses me a lot is if 5000 rows dataframe is small, why passing 5000 rows in pool object will trigger this error? https://stackoverflow.com/questions/47776486/python-struct-error-i-format-requires-2147483648-number-2147483647

BTW, I ran this python script using the PBS system on an HPC facility. The process number of the pool object is the vacant CPU number minus 1.

Thanks again for your time!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ahcub

ahcub commented May 25, 2020

Copy link
Copy Markdown
Contributor Author

Another thing confuses me a lot is if 5000 rows dataframe is small, why passing 5000 rows in pool object will trigger this error? https://stackoverflow.com/questions/47776486/python-struct-error-i-format-requires-2147483648-number-2147483647

this is probably because the python version on the system is not updated.

but to answer how 5k rows can be more than 2GB big I would probably need to see an example of the data.

For 100mil rows or any other amount, I guess I would recommend making chunks of 100-500 MB big (unzipped).
And if you are interested in processing huge amounts of data on a regular basis tho, I would recommend trying to find a solution for dealing with big data on a cluster.

if you want we can go through the code and the data on the call, and try to fix an issue you are having, together.
you can find my email on my github profile page.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@yangyxt

yangyxt commented May 25, 2020

Copy link
Copy Markdown

Another thing confuses me a lot is if 5000 rows dataframe is small, why passing 5000 rows in pool object will trigger this error? https://stackoverflow.com/questions/47776486/python-struct-error-i-format-requires-2147483648-number-2147483647

this is probably because the python version on the system is not updated.

but to answer how 5k rows can be more than 2GB big I would probably need to see an example of the data.

For 100mil rows or any other amount, I guess I would recommend making chunks of 100-500 MB big (unzipped).
And if you are interested in processing huge amounts of data on a regular basis tho, I would recommend trying to find a solution for dealing with big data on a cluster.

if you want we can go through the code and the data on the call, and try to fix an issue you are having, together.
you can find my email on my github profile page.

Dear ahcub,
Thank you for being willing to solve this issue with me. I'll find send an email to u and show you the code and the data. Thanks again!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Huanle Huanle mentioned this pull request Sep 28, 2020
long-read data causing memory/batch issue novoalab/EpiNano#59
Closed
@tomMoral tomMoral mentioned this pull request Oct 7, 2020
struct.error when each process sending back a large object joblib/joblib#1113
Closed
@marcus1487 marcus1487 mentioned this pull request Dec 16, 2020
Aggregation step is paused shortly after beginning nanoporetech/megalodon#76
Closed
@michalk8 michalk8 mentioned this pull request Jan 12, 2021
[FEATURE NAME] cr.pl.cluster_lineage throws MaybeEncodingError: Error sending result with many cells scverse/cellrank#460
Closed
@mkuchnik mkuchnik mentioned this pull request Apr 7, 2021
[Dataset Creation] Python Bug: 'i' format requires -2147483648 <= number <= 2147483647 facebookresearch/dlrm#172
Closed
@dennev dennev mentioned this pull request Nov 9, 2026
multiprocessing: on Windows, a message over 4,294,967,247 bytes raises OSError and breaks a waiting receiver #161668
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pitrou pitrou pitrou approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@ahcub @bedevere-bot @pitrou @yangyxt @the-knights-who-say-ni
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.