Is it advisable to avoid variable shadowing when using Candle?

[chenwanqq]

Considering following facts:

• Rust doesn't have a garbage collector.
• Variable shadowing does not release memory (drop) within the same scope.
• If track_op() is true, applying an operator to a tensor creates a new result tensor that holds a copy of the original tensor(s).

candle/candle-core/src/tensor.rs

Lines 675 to 682 in 2b10aaa

	pub fn elu(&self, alpha: f64) -> Result<Self> {
	if self.elem_count() == 0 {
	return Ok(self.clone());
	}
	let storage = self.storage().elu(self.layout(), alpha)?;
	let op = BackpropOp::new1(self, |t| Op::Elu(t, alpha));
	Ok(from_storage(storage, self.shape(), op, false))
	}

candle/candle-core/src/op.rs

Lines 877 to 884 in 2b10aaa

	pub(crate) fn new1(arg: &Tensor, f: impl Fn(Tensor) -> Op) -> Self {
	let op = if arg.track_op() {
	Some(f(arg.clone()))
	} else {
	None
	};
	Self(op)
	}

Many current model implementations are 'suspicious' regarding memory usage.

Compare the following two pseudo codes:

Code 1:

let mut x = some_big_tensor;
x = x.some_op(x)?; // The original x will be dropped here.

Code 2:

let x = some_big_tensor;
let x = x.some_op(x)?; // The original x is shadowed, but not dropped.

If my understanding is correct (although I haven't measured the actual memory usage), Code 2 will keep both original x and new x in memory because the original x is not dropped. It would be even worse if x is marked as variable in training scenario. In essence, variable shadowing could potentially lead to a significant increase in memory usage.

Models like LLaMA, Mistral, and LLaVA (which I have implemented) often rely on variable shadowing. If this issue is indeed valid, I believe numerous code segments should be revised, and a warning about this should be issued.

[EricLBuehler]

Hi @chenwanqq, I think that you are correct. Rust supports shadowing, and this does prevent dropping. See this playground or the below:

struct Tensor(u32);

impl Tensor {
    fn new(id: u32) -> Self {
        Self(id)
    }
}

impl Drop for Tensor {
    fn drop(&mut self) {
        println!("Dropping {:?}", self.0);
    }
}

fn main() {
    let x = Tensor::new(0);
    println!("Created 0");
    let x = Tensor::new(1);
    println!("Created 1");
}

What we want/expect is the following output, indicating that Tensor(0) is dropped when Tensor(1) is assigned to x:

Created 0
Dropping 0
Created 1
Dropping 1

However, this gives the below output:

Created 0
Created 1
Dropping 1
Dropping 0

This means that the shadowed tensor will remain in scope. There is a clippy lint for this which may be helpful to find/fix these occurances.