Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First Serializable implementation to support remoting of h1.Model over the wire #19

Closed
wants to merge 2 commits into from

Conversation

ctn
Copy link
Contributor

@ctn ctn commented Aug 23, 2020

.. we still have to make sure the offending (Keras) models are deleted prior to serialization, so that they don't cause exception on the other side.

.. to support remoting of Models over the wire. The main problem occurs when we pre-instantiate Keras models within h1.Models, and the latter has to be serialized for parallel execution. The contained Keras models do not serialize properly, and exceptions are thrown at the remote side as a result.
Copy link
Contributor

@nqbao nqbao left a comment

Choose a reason for hiding this comment

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

thank you @ctn . I and @thangtp can take the serializable implementation from here.

h1st/core/serializable.py Show resolved Hide resolved
h1st/core/model.py Show resolved Hide resolved
words = full_class_name.split(".")
module_name = ".".join(words[:-1])
class_name = words[-1:][0]
the_class = getattr(sys.modules[module_name], class_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

It is safer to use importlib to retrieve the module. we can also move this to util

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed re Utils.

h1st/core/serializable.py Show resolved Hide resolved
thangtp
thangtp previously approved these changes Aug 24, 2020
@thangtp thangtp self-requested a review August 24, 2020 17:00
@ctn
Copy link
Contributor Author

ctn commented Sep 6, 2020

On hold. See milestone info.

@ctn ctn added this to the 20Q3-M1A milestone Sep 6, 2020
@ghost ghost added the P3 Needs to be resolved within 30 days label Sep 22, 2020
@Shiti
Copy link
Contributor

Shiti commented Aug 4, 2021

This was being done to integrate training using Ray. This code is out of sync and a separate PR should be created with the required changes.

@Shiti Shiti closed this Aug 4, 2021
@TheVinhLuong102 TheVinhLuong102 deleted the serializable branch August 20, 2021 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 Needs to be resolved within 30 days
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants