search

microsoft / knossos-ksc

Compiler with automatic differentiation
Other
45 stars 10 forks source link

Remove allocating tensor constructor #995

Closed dcrc2 closed 2 years ago

dcrc2 commented 2 years ago

This PR tidies the tensor class by removing the constructor which took an allocator* argument. The existing function tensor::create is used instead.

The reason for doing this is to emphasise the fact that tensor does not own its data. I think it's potentially confusing to have a constructor which does allocation, but not to have any deallocation in the destructor. Additionally, create is more greppable.

toelli-msft commented 2 years ago

MNIST CNN seems to have a problem compiling with this.

dcrc2 commented 2 years ago

MNIST CNN seems to have a problem compiling with this.

Oops, yes sorry. I can fix this by rebasing onto #994, but will probably wait until that's been merged. There won't be any code changes needed in this PR so it's still OK to review at this point.

toelli-msft commented 2 years ago

Looks like this is removing the allocator constructor because its functionality is already provided by ::create. Good idea!