Don't use reader in ModelGraph and layers

[vloncar]

Description

Currently, adding weights data happens during initialization of layers by using the supplied reader. This is a relic of old design that required splitting this from converter. Upcoming ONNX support and nested model require this step to be done during conversion. This PR removes reader from ModelGraph and its subsequent use to read weights data. This is now moved to converters themselves and weights data is available as an attribute.

Type of change

• [x] (not exactly) New feature (non-breaking change which adds functionality)

Tests

This just changes the existing functionality so existing tests should verify that it works as expected

Checklist

• [x] I have read the guidelines for contributing.
• [x] I have commented my code, particularly in hard-to-understand areas.
• [x] I have made corresponding changes to the documentation.
• [x] My changes generate no new warnings.
• [x] I have installed and run pre-commit on the files I edited or added.

[jmitrevs]

Is this ready for review? My initial impression is positive, but I need to look more carefully.

[vloncar]

It is ready for review. Note that by accident I left in a definition of KerasNestedFileReader in keras_to_hls.py that is not used by the rest of the code. The followup PR that adds support for nested models makes use of it. I'll make that PR once this one is merged.

[jmitrevs]

onnx_to_hls, pytorch_to_hls, and tf_to_hls still pass the reader to ModelGraph. They will no longer work with this change. Are they not changed because others (e.g. me for ONNX) will fix this and changing it will make a merge conflict?

By the way, what's the status of tf_to_hls? Does anyone support it?

[vloncar]

We agreed that the changes to onnx and pytorch will come after this merge, since both need to be updated. tf_to_hls is not supported anymore, no one uses it, it relies on old v1 compatibility so it is not worth maintaining in its current form.

[jmitrevs]

Sounds good. I will make a few more checks and likely merge this. As a separate PR we may consider removing tf_to_hls afterwards.

[jmitrevs]

Is there a convention enforced with _data?