deephealthproject / eddl

European Distributed Deep Learning (EDDL) library. A general-purpose library initially developed to cover deep learning needs in healthcare use cases within the DeepHealth project.
https://deephealthproject.github.io/eddl/
MIT License
34 stars 10 forks source link

Missing deletes #293

Closed CostantinoGrana closed 2 years ago

CostantinoGrana commented 3 years ago

First

In ConvolDescriptor::build(Tensor *A) I see this piece of code, which is pretty hard for me to understand:

        // Tensor with variable shared ptr, delete create ptr
        gpuI=new Tensor(vector<int>{r*c,kc*kr*kz}, I->device);
        gpu_delete_tensor(gpuI->gpu_device,gpuI->ptr);
        gpuO=new Tensor(vector<int>{z,r*c}, I->device);
        gpu_delete_tensor(gpuI->gpu_device,gpuO->ptr);
        //gpu_delete_tensor(gpuI->gpu_device,gpuOB->ptr);
        gpuD=new Tensor(vector<int>{z,r*c}, I->device);
        gpu_delete_tensor(gpuI->gpu_device,gpuD->ptr);
        gpuK=new Tensor(vector<int>{z,kc*kr*kz}, I->device);
        gpu_delete_tensor(gpuI->gpu_device,gpuK->ptr);
        gpugK=new Tensor(vector<int>{z,kc*kr*kz}, I->device);
        gpu_delete_tensor(gpuI->gpu_device,gpugK->ptr);

because I couldn't get the meaning of the gpu_delete_tensor() function. What instead I'm pretty sure is missing is the corresponding delete for the Tensor objects thus allocated with new. The destructor seems not to delete them.

Second

I believe that the destructor of Net is missing the delete of the single nets in the snets vector. Maybe this can be fixed with a delete snets[i]; at line 183.

MicheleCancilla commented 3 years ago

Third

The function cuDNN_environment_initialization in _gpuconv.cu allocates several arrays which (I guess) should be deleted:

cudnnConvolutionFwdAlgoPerf_t * perfResults = new cudnnConvolutionFwdAlgoPerf_t [requestedAlgoCount];
...
cudnnConvolutionBwdFilterAlgoPerf_t * perfResultsbwf = new cudnnConvolutionBwdFilterAlgoPerf_t [requestedAlgoCount];
...
cudnnConvolutionBwdDataAlgoPerf_t * perfResults_d = new cudnnConvolutionBwdDataAlgoPerf_t [requestedAlgoCount];
RParedesPalacios commented 3 years ago

We have checked and memory is ok.

These lines: gpuD=new Tensor(vector{z,r*c}, I->device); gpu_delete_tensor(gpuI->gpu_device,gpuD->ptr);

Create a tensor object that will share the memory created in other place but I need to encapsulate that memory also in a Tensor object (struct). So I create the tensor object I will need (main attributes) but delete the memory created in this new Tensor

Snets are freed

regarding CudNN not sure, I didn't write that part (yes unbelievable :) )

Thanks

CostantinoGrana commented 3 years ago

So, the memory to which gpuI points is not freed, but it's only a small container which doesn't point to any memory, right?

In the Net destructor happens the same, because the content of each snet[i] is deleted, but the snet[i] is not. Why not deleting snets[i]?