tensor_db get_aggregated_tensor duplicates code, returns tuple in corner case

[msheller]

Describe the bug
Calling get_aggregated_tensor on a tensor that has already been aggregated returns a tuple of (tensor, {}).

Expected behavior
I would expect that get_aggregated_tensor would call get_tensor_from_cache first, then aggregate if no tensor is found, thus using the same logic as the get_tensor_from_cache method (which doesn't return a tuple).

Additional context
Unclear why we use the method names "get_tensor_from_cache" and "cache_tensor" instead of "get_tensor" and "add_tensor". Legacy names, maybe?

[tanwarsh]

@msheller, This is an intended behaviour as aggregator calling get_aggregated_tensor on a tensor that has already been aggregated should not occur in normal conditions. Is there a special use case which requires aggregation more then once?

In my opinion, instead of throwing an error that causes the experiment to stop abruptly, we should handle this scenario gracefully by stating the reason and marking the experiment as unsuccessful.

or we can still allow experiment to continue for which the changes are done in #1121.
@rahulga1 @teoparvanov @psfoley @MasterSkepticista @theakshaypant what are your thoughts on this?

[MasterSkepticista]

Instead of maintaining internal call state, and/or raising exceptions, the function should obey idempotence - returning the same result on every call.

Unless there are situations where calling any function more than once is violation of a state in a state machine (which I do not believe to be the case for calling get_aggregated_tensor), idempotence is the way to go.

[tanwarsh]

idempotence

Ya, changes in #1121 will make it idempotent which is suggested in my previous comment.

[teoparvanov]

Suggesting to close this issue, as it has been addressed in OpenFL v1.6 as part of PR #996