Skip to content

rocAL Deserialize PR 1#14

Open
fiona-gladwin wants to merge 98 commits intofgladwin:gf/serialize_mergefrom
fiona-gladwin:fg/deser_pr1
Open

rocAL Deserialize PR 1#14
fiona-gladwin wants to merge 98 commits intofgladwin:gf/serialize_mergefrom
fiona-gladwin:fg/deser_pr1

Conversation

@fiona-gladwin
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces deserialization support for rocAL by implementing a factory pattern for dynamic node creation. The changes enable runtime node registration and instantiation, which is essential for deserializing pipeline graphs.

  • Adds NodeFactory singleton class with registration and creation methods for loader and augmentation nodes
  • Introduces REGISTER_LOADER_NODE and REGISTER_NODE macros for automatic node registration
  • Extends EnumRegistry with runtime enum conversion capabilities from integer values
  • Updates loader node classes to properly override get_loader_module() method

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
rocAL/include/pipeline/node.h Introduces NodeFactory singleton pattern, registration macros, and virtual get_loader_module() method for deserialization support
rocAL/include/pipeline/enum_registry.h Adds convertIntToEnum() method and name-to-converter mapping for runtime enum conversion
rocAL/include/loaders/loader_module.h Updates include to use specific metadata reader header
rocAL/include/loaders/video/node_video_loader_single_shard.h Adds override specifier to get_loader_module()
rocAL/include/loaders/video/node_video_loader.h Adds override specifier to get_loader_module()
rocAL/include/loaders/image/node_numpy_loader_single_shard.h Adds override specifier to get_loader_module()
rocAL/include/loaders/image/node_numpy_loader.h Adds override specifier to get_loader_module()
rocAL/include/loaders/image/node_image_loader_single_shard.h Adds override specifier to get_loader_module()
rocAL/include/loaders/image/node_image_loader.h Adds override specifier to get_loader_module() and registers node with REGISTER_LOADER_NODE macro
rocAL/include/loaders/image/node_fused_jpeg_crop_single_shard.h Adds override specifier to get_loader_module()
rocAL/include/loaders/image/node_fused_jpeg_crop.h Adds override specifier to get_loader_module()
rocAL/include/loaders/image/node_cifar10_loader_single_shard.h Adds override specifier to get_loader_module()
rocAL/include/loaders/image/node_cifar10_loader.h Adds override specifier to get_loader_module()
rocAL/include/augmentations/color_augmentations/node_brightness.h Registers BrightnessNode using REGISTER_NODE macro

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread rocAL/include/pipeline/enum_registry.h Outdated
Comment thread rocAL/include/pipeline/node.h Outdated
Comment thread rocAL/include/pipeline/node.h Outdated
Comment thread rocAL/include/pipeline/node.h Outdated
Comment thread rocAL/include/pipeline/node.h Outdated
Comment thread rocAL/include/pipeline/node.h Outdated
Comment thread rocAL/include/pipeline/node.h Outdated
Comment thread rocAL/include/pipeline/node.h Outdated
Comment thread rocAL/include/pipeline/node.h Outdated
Comment thread rocAL/include/pipeline/node.h Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread rocAL/include/pipeline/enum_registry.h
Comment thread rocAL/include/loaders/image/node_image_loader.h Outdated
Comment thread rocAL/include/augmentations/color_augmentations/node_brightness.h Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread rocAL/include/pipeline/enum_registry.h
#include "loaders/loader_module.h"
#include "pipeline/tensor.h"
#include "pipeline/argument.h"
#include "pipeline/node_factory.h"
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This include appears unnecessary. The Node class doesn't use NodeFactory in its declaration, and the registration macros (REGISTER_LOADER_NODE, REGISTER_NODE) are used in .cpp files, not headers. Including node_factory.h here creates a circular dependency risk (node_factory.h forward-declares Node, and node.h includes node_factory.h). Consider removing this include and only including node_factory.h in the source files that use the registration macros.

Suggested change
#include "pipeline/node_factory.h"

Copilot uses AI. Check for mistakes.
fgladwin and others added 6 commits November 20, 2025 12:57
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ocs/sphinx (ROCm#418)

Bumps [rocm-docs-core[api_reference]](https://github.com/ROCm/rocm-docs-core) from 1.29.0 to 1.30.0.
- [Release notes](https://github.com/ROCm/rocm-docs-core/releases)
- [Changelog](https://github.com/ROCm/rocm-docs-core/blob/develop/CHANGELOG.md)
- [Commits](ROCm/rocm-docs-core@v1.29.0...v1.30.0)

---
updated-dependencies:
- dependency-name: rocm-docs-core[api_reference]
  dependency-version: 1.30.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…ocs/sphinx (ROCm#423)

Bumps [rocm-docs-core[api_reference]](https://github.com/ROCm/rocm-docs-core) from 1.30.0 to 1.30.1.
- [Release notes](https://github.com/ROCm/rocm-docs-core/releases)
- [Changelog](https://github.com/ROCm/rocm-docs-core/blob/develop/CHANGELOG.md)
- [Commits](ROCm/rocm-docs-core@v1.30.0...v1.30.1)

---
updated-dependencies:
- dependency-name: rocm-docs-core[api_reference]
  dependency-version: 1.30.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
fiona-gladwin and others added 30 commits December 19, 2025 11:53
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…OCm#411)

* Use enums present in commons.h

Remove the usage of API enums in internal node files

* Remove unused include rocal_api_types.h

* Introduce EnumRegistry

Add support to register all internal enums

* Introduce Argument class

Creates an argument instance for each arg passed in the Node
Handles different argument types i.e parameters, vectors, enums and predefined types

* Introduce PipelineOperator class

* Add support to create and return node name and tensor name

* Add support in MasterGraph to store details of PipelineOperators

* Add support to store the argument details in the node for brightness and ImageLoaderNode

* Introduce PipelineSerializer

Add support to serialize the args and pipeline details
Add support to serialize the tensors.

* Introduce external rocalSerialize API

* Add support to serialize parameter arguments

* Remove redundant static cast

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Remove semicolon for enum macro

* Modify name of macro

* Change enums to scoped enums in commons.h

* Code reorganization

Create single constructor with check for different data types in Argument class

* Minor fix

* Remove unused includes

* Add compiler keywords

* Update rocAL/include/pipeline/argument.h

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update rocAL/include/pipeline/argument.h

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Minor change

* Update rocAL/include/pipeline/argument.h

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update rocAL/include/pipeline/argument.h

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Minor change

* Fix shared pointer fetching

* Minor fix

* Minor fix

* Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update CHANGELOG.md

* Update CHANGELOG.md

* Add all serialization code to pipeline serializer

Remove the use of pipeline operator

* Revert fix

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update CHANGELOG.md

* Add comments and doc description

* Support to serialize to file

* Update CMakeLists version for rocAL

* Update CHANGELOG

* Update CHANGELOG.md

* Minor change

* Minor changes

* Update CHANGELOG.md

Co-authored-by: spolifroni-amd <Sandra.Polifroni@amd.com>

* Add const qualifier

* Add const correctness to get args list

* Add const correctness to get args list

* Minor change

* Add const correctness to get args list

* Minor change

* Return serialized string reference from MasterGraph

* Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Obtain serialized string size as ptr

* Minor change

* Add reset for pipeline serializer

* Add cstring include

* Minor change

* Update CHANGELOG

* Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Minor changes

* Add empty lines in between functions

* Remove Params proto

* Add params proto

* Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Minor changes

* Update CHANGELOG

* Update CHANGELOG

* Change rocal proto file to proto2 features

* Generate UID for tensors from MasterGraph

* Minor change

* Resolve copilot review comments

* Minor change

* Resolve copilot review comments

* Use uint64 in proto for pipeline members

* Resolve review comments

* Minor changes - review comments

* Add error check statements

* Revert input and outputs

* Use uint64 in proto for pipeline members

* Revert "Revert input and outputs"

This reverts commit 72ff657.

* Minor changes

* Return tensor name as reference

* Remove type string from InputOutput proto

* Add support to store and fetch seed value in proto

* Remove serialize to file

* Update CHANGELOG

* Add period in throw statements

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Fiona Gladwin <121928245+fgladwin@users.noreply.github.com>
Co-authored-by: Sundar Rajan Vaithiyanathan <99159823+SundarRajan28@users.noreply.github.com>
Co-authored-by: Kiriti Gowda <kiritigowda@gmail.com>
Co-authored-by: spolifroni-amd <Sandra.Polifroni@amd.com>
Co-authored-by: Lakshmi Kumar <lakshmi.kumar@amd.com>
…ocs/sphinx (ROCm#428)

Bumps [rocm-docs-core[api_reference]](https://github.com/ROCm/rocm-docs-core) from 1.31.0 to 1.31.1.
- [Release notes](https://github.com/ROCm/rocm-docs-core/releases)
- [Changelog](https://github.com/ROCm/rocm-docs-core/blob/develop/CHANGELOG.md)
- [Commits](ROCm/rocm-docs-core@v1.31.0...v1.31.1)

---
updated-dependencies:
- dependency-name: rocm-docs-core[api_reference]
  dependency-version: 1.31.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Kiriti Gowda <kiritigowda@gmail.com>
This reverts commit f821ce6.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Fix Lens Correction API

* Fix Webdataset python unit test

* Fix Vignette python test case

* Mark Fog as random augmentation

* Add COCO test case for ResizeMirrorNormalize

* Fix the error statements

* Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Revert change

* Add Fog to randomised augmentation

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Kiriti Gowda <kiritigowda@gmail.com>
Co-authored-by: Lakshmi Kumar <lakshmi.kumar@amd.com>
* Use enums present in commons.h

Remove the usage of API enums in internal node files

* Remove unused include rocal_api_types.h

* Introduce EnumRegistry

Add support to register all internal enums

* Introduce Argument class

Creates an argument instance for each arg passed in the Node
Handles different argument types i.e parameters, vectors, enums and predefined types

* Introduce PipelineOperator class

* Add support to create and return node name and tensor name

* Add support in MasterGraph to store details of PipelineOperators

* Add support to store the argument details in the node for brightness and ImageLoaderNode

* Introduce PipelineSerializer

Add support to serialize the args and pipeline details
Add support to serialize the tensors.

* Introduce external rocalSerialize API

* Add support to serialize parameter arguments

* Introduce serialize tests

* Remove redundant static cast

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Add support to dump images using OpenCV

* Update CMakeLists

* Update copyright

* Remove semicolon for enum macro

* Modify name of macro

* Change enums to scoped enums in commons.h

* Code reorganization

Create single constructor with check for different data types in Argument class

* Minor fix

* Remove unused includes

* Add compiler keywords

* Update rocAL/include/pipeline/argument.h

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update rocAL/include/pipeline/argument.h

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Minor change

* Update rocAL/include/pipeline/argument.h

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update rocAL/include/pipeline/argument.h

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Minor change

* Fix shared pointer fetching

* Minor fix

* Minor fix

* Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update CHANGELOG.md

* Update CHANGELOG.md

* Add all serialization code to pipeline serializer

Remove the use of pipeline operator

* Revert fix

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update CHANGELOG.md

* Add comments and doc description

* Support to serialize to file

* Update CMakeLists version for rocAL

* Update CHANGELOG

* Update CHANGELOG.md

* Minor change

* Minor changes

* Update CHANGELOG.md

Co-authored-by: spolifroni-amd <Sandra.Polifroni@amd.com>

* Add const qualifier

* Add const correctness to get args list

* Add const correctness to get args list

* Minor change

* Add const correctness to get args list

* Minor change

* Return serialized string reference from MasterGraph

* Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Obtain serialized string size as ptr

* Minor change

* Add reset for pipeline serializer

* Add cstring include

* Minor change

* Update CHANGELOG

* Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Minor changes

* Add empty lines in between functions

* Remove Params proto

* Add params proto

* Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Minor changes

* Fix serialize test

* Add CMakeLists for serialization tests

* Use random brightness API

* Add support to build serialization tests in CMakeLists

* Update CHANGELOG

* Update CHANGELOG

* Update CHANGELOG.md

* Change rocal proto file to proto2 features

* Generate UID for tensors from MasterGraph

* Minor change

* Resolve copilot review comments

* Minor change

* Resolve copilot review comments

* Use uint64 in proto for pipeline members

* Resolve review comments

* Minor changes - review comments

* Add error check statements

* Revert input and outputs

* Use uint64 in proto for pipeline members

* Revert "Revert input and outputs"

This reverts commit 72ff657.

* Minor change

* Minor changes

* Return tensor name as reference

* Remove type string from InputOutput proto

* Add support to store and fetch seed value in proto

* Remove serialize to file

* Update CHANGELOG

* Add period in throw statements

* Fix opencv include in serialize test

* Remove OpenCV 3 usage in serialization test

* Update README with prerequisites

* Remove Opencv4 definitions

* Minor changes

* Remove extra lines

* Minor change

* Revert "Minor change"

This reverts commit f821ce6.

* Minor change

* Fix CMake

* Remove compression params in serialization tests

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Fiona Gladwin <121928245+fgladwin@users.noreply.github.com>
Co-authored-by: Sundar Rajan Vaithiyanathan <99159823+SundarRajan28@users.noreply.github.com>
Co-authored-by: Kiriti Gowda <kiritigowda@gmail.com>
Co-authored-by: spolifroni-amd <Sandra.Polifroni@amd.com>
Co-authored-by: Lakshmi Kumar <lakshmi.kumar@amd.com>
* Use enums present in commons.h

Remove the usage of API enums in internal node files

* Remove unused include rocal_api_types.h

* Introduce EnumRegistry

Add support to register all internal enums

* Introduce Argument class

Creates an argument instance for each arg passed in the Node
Handles different argument types i.e parameters, vectors, enums and predefined types

* Introduce PipelineOperator class

* Add support to create and return node name and tensor name

* Add support in MasterGraph to store details of PipelineOperators

* Add support to store the argument details in the node for brightness and ImageLoaderNode

* Introduce PipelineSerializer

Add support to serialize the args and pipeline details
Add support to serialize the tensors.

* Introduce external rocalSerialize API

* Add support to serialize parameter arguments

* Introduce serialize tests

* Remove redundant static cast

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Add support to dump images using OpenCV

* Update CMakeLists

* Update copyright

* Remove semicolon for enum macro

* Modify name of macro

* Change enums to scoped enums in commons.h

* Code reorganization

Create single constructor with check for different data types in Argument class

* Minor fix

* Remove unused includes

* Add compiler keywords

* Update rocAL/include/pipeline/argument.h

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update rocAL/include/pipeline/argument.h

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Minor change

* Update rocAL/include/pipeline/argument.h

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update rocAL/include/pipeline/argument.h

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Minor change

* Fix shared pointer fetching

* Minor fix

* Minor fix

* Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update CHANGELOG.md

* Update CHANGELOG.md

* Add all serialization code to pipeline serializer

Remove the use of pipeline operator

* Revert fix

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update CHANGELOG.md

* Add comments and doc description

* Support to serialize to file

* Update CMakeLists version for rocAL

* Update CHANGELOG

* Update CHANGELOG.md

* Minor change

* Minor changes

* Update CHANGELOG.md

Co-authored-by: spolifroni-amd <Sandra.Polifroni@amd.com>

* Add const qualifier

* Add const correctness to get args list

* Add const correctness to get args list

* Minor change

* Add const correctness to get args list

* Minor change

* Return serialized string reference from MasterGraph

* Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Obtain serialized string size as ptr

* Minor change

* Add reset for pipeline serializer

* Add cstring include

* Minor change

* Update CHANGELOG

* Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Minor changes

* Add empty lines in between functions

* Remove Params proto

* Add params proto

* Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Minor changes

* Fix serialize test

* Add CMakeLists for serialization tests

* Use random brightness API

* Add support to build serialization tests in CMakeLists

* Update CHANGELOG

* Update CHANGELOG

* Update CHANGELOG.md

* Change rocal proto file to proto2 features

* Generate UID for tensors from MasterGraph

* Minor change

* Resolve copilot review comments

* Minor change

* Resolve copilot review comments

* Use uint64 in proto for pipeline members

* Resolve review comments

* Minor changes - review comments

* Add error check statements

* Revert input and outputs

* Use uint64 in proto for pipeline members

* Revert "Revert input and outputs"

This reverts commit 72ff657.

* Minor change

* Minor changes

* Return tensor name as reference

* Remove type string from InputOutput proto

* Add python support for rocAL serialize

* Accept filename as input

* Introduce python serialization test file

* Add support to store and fetch seed value in proto

* Remove serialize to file

* Register copy node

* Add serialization support for ImageLoaderSingleShardNode

* Revert "Register copy node"

This reverts commit f904406.

* Update python test file

* Minor changes

* Fix the size of char buffer and other minor fixes

* Update CHANGELOG

* Add period in throw statements

* Update CHANGELOG.md

* Fix opencv include in serialize test

* Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Remove OpenCV 3 usage in serialization test

* Update README with prerequisites

* Remove Opencv4 definitions

* Minor changes

* Remove extra lines

* Minor change

* Revert "Minor change"

This reverts commit f821ce6.

* Minor change

* Minor change

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Fiona Gladwin <121928245+fgladwin@users.noreply.github.com>
Co-authored-by: Sundar Rajan Vaithiyanathan <99159823+SundarRajan28@users.noreply.github.com>
Co-authored-by: Kiriti Gowda <kiritigowda@gmail.com>
Co-authored-by: spolifroni-amd <Sandra.Polifroni@amd.com>
Co-authored-by: Lakshmi Kumar <lakshmi.kumar@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants