thu-ml/tianshou

Batch: deprecate setattr

Opened this issue · 1 comments

There is no need to ever do something like

b = Batch(...)
b.new_attr = [1, 2, 3]

Batch behaves very surprisingly when this is done: sequences are cast to arrays, dicts are cast to Batch, datatypes and even lengths of arrays may change.

This can be solved by finding where setattr is being used (e.g., by adding a print statement to the implementation and running some examples), add a new method a la add_sequence, or even not support adding sequences post-hoc at all, e.g., in favor of creating a new batch with a method like with_added_sequence.

This is actually an integral part of, for example, process_fn where derived values such as n-step returns are computed and added to the current batch of data.

end_flag = buffer.done.copy()
end_flag[buffer.unfinished_index()] = True
target_q = _nstep_return(rew, end_flag, target_q, indices, gamma, n_step)
batch.returns = to_torch_as(target_q, target_q_torch)

I guess the same should happen to __setitem__? Same parsing of values is happening there. Actually it happens already in __init__...