search

IBM / aihwkit

IBM Analog Hardware Acceleration Kit
https://aihwkit.readthedocs.io
Apache License 2.0
349 stars 145 forks source link

Updated drift compensation mechanism to use `is_perfect=True` when determining the reference readout #674

Open coreylammie opened 1 month ago

coreylammie commented 1 month ago

Description

Updated drift compensation mechanism to use is_perfect=True when determining the reference readout. Previously, all non-idealities (as set by the corresponding rpu_config) were modeled, resulting in sub-optimal drift compensation scales being computed in some scenarios, e.g., where the output noise is sufficiently large.

maljoras commented 1 month ago

@coreylammie, the drift compensation is on purpose using the rpu config of the tile as this is the realistic situation. In reality, how can you achieve a perfect reference readout, as there always will be noise when reading the analogue devics? I don't think you should change this default behaviour as it would then make the simulation unrealistic and wrong.

maljoras commented 1 month ago

If you want perfect readout during drift compensation, then you should explicitly write a new drift compensation. This should be a special case as this behaviour is wrong in most natural cases.

maljoras commented 1 month ago

This PR will also invalidate the papers we have written using this standard drift where the correct way was used to estimate the drift compensation in a noisy manner. So please take this PR back. If you need a perfect compensation for some reasons, write a special drift compensation class by deriving from the default one.

maljoras commented 1 month ago

Also the implementation seems to permanently set the rpu config to perfect, which is then always using the perfect case, which is even wrong, if you only want to use a noise free reference. It will then use a completely noise free drift. That's completely wrong.

maljoras commented 1 month ago

Please don't change the default drift class in this manner. Use a special drift class if you need this special behaviour for some reasons.

maljoras commented 1 month ago

I am about to close this PR. Please submit a new one where you modify your PR accordingly by deriving a new drift compensation class from the default class and not changing the behaviour of the default class. Also, as stated above, it seems that the rpu configuration is permanently changed which is wrong in any case.

maljoras commented 1 month ago

Btw, if you want the exact weights of a tile for the reference computation of your special drift compensation class, simply use tile.get_weights(realistic=False) instead of changing the forward read to is_perfect=True