Makes prediction work on GPUs

[dirkgr]

When you use numpy.zeros to create the embed_arr, you can't later add it to embed_vector, because embed_vector might not be a numpy array. This re-works the code such that the array type of embed_vector is preserved all the way through.

I stole this approach from explosion/spaCy#3362. Thanks, @danielkingai2!

[dirkgr]

To help those who Google, the error message you get ends with

  File "neuralcoref.pyx", line 596, in neuralcoref.neuralcoref.NeuralCoref.__call__
  File "neuralcoref.pyx", line 723, in neuralcoref.neuralcoref.NeuralCoref.predict
  File "neuralcoref.pyx", line 913, in neuralcoref.neuralcoref.NeuralCoref.get_mention_embeddings
  File "neuralcoref.pyx", line 904, in neuralcoref.neuralcoref.NeuralCoref.get_average_embedding
  File "cupy/core/core.pyx", line 1238, in cupy.core.core.ndarray.__array_ufunc__
  File "cupy/core/_kernel.pyx", line 816, in cupy.core._kernel.ufunc.__call__
  File "cupy/core/_kernel.pyx", line 99, in cupy.core._kernel._preprocess_args
TypeError: Unsupported type <class 'numpy.ndarray'>

Though it does seem there are other problems too when you run on GPU.

[dirkgr]

I fixed all the other problems I am aware of at this point. On my machine, it runs about 8x faster on one GPU.

[thomwolf]

Maybe we can avoid this dependency check by relying on Thinc for that.

[dirkgr]

How do you feel about this?

def to_numpy(a):
    if thinc.neural.util.is_cupy_array(a):
        import cupy
        return cupy.asnumpy(a)
    else:
        return a

That way there isn't a conditional import, but we still have to import cupy. I'm not that familiar with thinc, but the thinc source does not use cupy.asnumpy() anywhere, so there probably isn't a good wrapper.

[thomwolf]

Why did you remove these?

[dirkgr]

This is all about removing the call to numpy.zeros(). Once I had replaced it with sum(), the code collapsed into just those two lines. Other than the location of the output vector, it should perform exactly the same way.

[HarshTrivedi]

@thomwolf Can you take a look at this PR? I am trying to use neuralcoref on a large corpus but w/o GPU it's taking too much of time.

[HarshTrivedi]

I could get it to work on GPU using this fix, thanks @dirkgr!

[jqueguiner]

Hi guys this is awesome work is it possible to merge the PR ?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[dirkgr]

Hi guys this is awesome work is it possible to merge the PR ?

Looks like the answer is "no".

[svlandeg]

Sorry for the late response to this. The PR closed automatically but I think this is valuable work so I reopened. We're probably going to work on a closer integration with spaCy & thinc too.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[jkhalsa-arabesque]

Any updates on this?

[m1]

Would love some updates on this!

[svlandeg]

I want to keep this PR open as the code may be useful for those who want to build from source and try this out.

However moving forward, when spaCy v.3 will be released, we'll update this code significantly to be compatible with Thinc 8. At that point, GPU support will be automatic...

[sreyemnayr]

For those trying to make the pipeline work with GPU support on spaCy > 2.1, here's the additional step for patching prior to installing from source (after activating your venv and pip installing spaCy):

git clone https://github.com/huggingface/neuralcoref.git
cd neuralcoref
git fetch origin pull/149/head:gpufix
git checkout gpufix
pip install -r requirements.txt
pip install -e .

[c0stya]

Any progress on spaCy v.3 integration?

[svlandeg]

You mean the v3 that was released yesterday? ;-)

It's definitely on our roadmap, but it's not the only thing we're working on ;-)

[LifeIsStrange]

@svlandeg Friendly ping :)

[svlandeg]

Hi! Please refer to #295 (comment) for more info :-)

[LifeIsStrange]

@svlandeg Thx for the update, I'm curious wether the updated implementation target the latest state of the art (cf #334 ) AKA 81% accuracy on Ontonotes (or at least 79% since the second paper is quite old already)