[Lookup] Relax input datatype constraints

[iksnagreb]

FINN and QONNX generally assume float32 as the container datatype of tensors. However, Gather/Lookup export with int64 tensors as the index input, which makes more sense, but is not properly handled by our compiler infrastructure. Instead of refactoring container datatype handling throughout the code base, for now it seems to be sufficient to disable some assertions or at least relax them to warnings. A proper refactoring of the Lookup operator might be necessary in the near future anyway.

[auphelia]

How come you needed to add this in StreamingFIFO as well?

[iksnagreb]

Hm, not exactly sure, I have seen this assertion fail in combination with Lookup. And I think, at least for simulation, it is indeed to restrictive in general: Almost always we assume tensors to be stored in float32 containers and usually QONNX attaches a datatype annotation (to represent the actual, quantized datatype) to this. However, some transformations, operators and also some numpy functions used by the infrastructure do not always keep these two concept as cleanly separated as they should (some numpy functions for example implicitly default to float64, we have already seen this as part of the RoundAndClipThresholds issue..., in this case, Lookup/Gather wants some int64 type, I guess). Whether the whole float32 container type thing really make sense or should be reworked to respect more numpy types is another discussion, but for now I think, anytime we are asserting something about the dtype, i.e., the container type, we should relax this to some warning alongside .astype(np.float32) to restore the expected behavior and just check whether the simulation still yields the expected results.

[auphelia]

Warnings have a tendency to be overlooked and this code changes the model like a transformation would. If I have your ok, I would rather suggest a temporary interpretation of the dtype as float32 for the execution but afterwards reverting this back to what the tensor dtype was. For the LookUp layer, I am ok with these changes for now because it is more contained and I believe that layer needs a bigger refactoring soon.

[iksnagreb]

This should not change the model, this is only part of the execute_node rtlsim infrastructure and if you look at the output side, it already does something similar, just without any warning/error:

finn/src/finn/custom_op/fpgadataflow/rtl/streamingfifo_rtl.py

Line 174 in 15c6e38

output = np.asarray([output], dtype=np.float32).reshape(*oshape)

And likewise the cppsim python/numpy fallback:

finn/src/finn/custom_op/fpgadataflow/rtl/streamingfifo_rtl.py

Line 131 in 15c6e38

output = np.asarray([output], dtype=np.float32).reshape(*exp_shape)

[auphelia]

Oh yes, I see! Thanks, I mistook inp for the tensor itself and not the input values.

[auphelia]

Thank you for the clarifications, looks good to me!