uber / petastorm

Petastorm library enables single machine or distributed training and evaluation of deep learning models from datasets in Apache Parquet format. It supports ML frameworks such as Tensorflow, Pytorch, and PySpark and can be used from pure Python code.

Apache License 2.0

1.78k stars 285 forks source link

Add a flag to factory methods to allow zmq copy buffers to be disabled #596

Closed dmcguire81 closed 4 years ago

dmcguire81 commented 4 years ago

Addresses #595

codecov[bot] commented 4 years ago

Codecov Report

Merging #596 into master will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #596   +/-   ##
=======================================
  Coverage   85.68%   85.68%           
=======================================
  Files          87       87           
  Lines        4994     4994           
  Branches      795      795           
=======================================
  Hits         4279     4279           
  Misses        578      578           
  Partials      137      137

Impacted Files	Coverage Δ
petastorm/workers_pool/process_pool.py	`92.70% <ø> (ø)`
petastorm/reader.py	`90.24% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1a617b6...9489b71. Read the comment docs.

dmcguire81 commented 4 years ago

I still owe you a test, here, so I'll get to this, today.

dmcguire81 commented 4 years ago

@selitvin let me know if this test works for you

selitvin commented 4 years ago

@dmcguire81: looks good. Let's fix whatever flake8 complaints about and land it.

dmcguire81 commented 4 years ago

@selitvin can you restart the Travis CI build? It looks like a brittle test is timing out, which seems unrelated.

selitvin commented 4 years ago

@selitvin can you restart the Travis CI build? It looks like a brittle test is timing out, which seems unrelated.

Hmmm. This is annoying. Did not see these timeouts before... Restarted.

selitvin commented 4 years ago

These test timeouts appear to be in the similar place as the hanging you observed with s3. Do you think they are related?

dmcguire81 commented 4 years ago

These test timeouts appear to be in the similar place as the hanging you observed with s3. Do you think they are related?

Could be. I'll pull you into the ongoing discussion with pyarrow and s3fs maintainers, since they asked for a reproducer that you may be able to build more quickly than me, if you're more familiar. Needing to work around this deadlock is also related to our discussion of parameterizing the FileSystem instance.

dmcguire81 commented 4 years ago

@selitvin https://github.com/dask/s3fs/issues/365