Repository navigation
Early deletion in gw_ml.F90 #37
Description
Line 123 in 8ef0f5a
Aren't we are deleting too early since the allocation of net_outputs is aliased by net_output_tensors?
Or does the call to torch_delete(net_output_tensors(1)) likely only destroy the PyTorch tensor wrapper, not the memory buffer it pointed to (which is net_outputs(:, i)). There is a check for zero outputs, so if this isn't satisfied wouldn't net_outputs be nonsense and therefore blow up? This doesn't tally with our observations (3 year runs were successful at the time).
If we moved to batching and pushed the deletions a little lower this wouldn't be a discussion anyway.
Maybe @jatkinson1000 can help with this.
Activity
jatkinson1000 commentedon Nov 11, 2025
I would have intuitively said so, yes. Originally I had it outside the loop.
However, they were moved inside the loop to fix the memory leak when working on the earliest stages of this work with Qiang, Pedram, and Joan, see b627d7d which was part of #14
I'll try and remember the exact reasoning for this and discussion we had at the time, but here is the reference for now.
ma595 commentedon Nov 11, 2025
Yes, I think this makes sense for the inputs, but deleting the outputs too early might lead to undefined behaviour.
jatkinson1000 commentedon Nov 11, 2025
Ahhh, right.
The base of the call in the C++ calls
delete t, wheretis theTorch::tensor.In theory, because of the shared memory this should free it in the eyes of C++ but the data should remain pointed at and usable by the Fortran. However, I agree that if C++ thinks it is free and started writing something else there this could cause issues...
It should also (possibly) not be needed at all now as the FTorch torch_tensor has a
finalizeto call the deletion when tensors go out of scope which should be the safest/cleanest way to handle this. This was added as part of autograd since the original DataWave work.