Skip to content
This repository has been archived by the owner on Sep 18, 2024. It is now read-only.

Serialization infrastructure V2 #4337

Merged
merged 19 commits into from
Dec 6, 2021

Conversation

ultmaster
Copy link
Contributor

@ultmaster ultmaster commented Nov 26, 2021

Description

This PR refines the serialization infrastructure as introduced in the previous release, and integrates it with Retiarii.

Adopt nni.load and nni.dump in most cases where serialization is used in NNI.

I think this PR is big enough. I'll leave the work of evaluator_fn (to support evaluator that doesn't need to wrap with serialize) in a future PR.

Checklist

  • test case
  • doc

How to test

See test/ut/sdk/test_serializer.py. Needs to test NAS multi-trial examples and integration tests.

@liuzhe-lz liuzhe-lz mentioned this pull request Nov 26, 2021
86 tasks
@ultmaster ultmaster linked an issue Nov 26, 2021 that may be closed by this pull request
@ultmaster ultmaster closed this Nov 29, 2021
@ultmaster ultmaster reopened this Nov 29, 2021
Copy link
Contributor

@J-shang J-shang left a comment

Choose a reason for hiding this comment

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

LGTM


Also it records extra information about where this object comes from. That's why it's called "trace".
When parameters of functions are received, it is first stored, and then a shallow copy will be passed to inner function.
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the meaning of "inner function"? is there example for it?

Choose a reason for hiding this comment

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

The inner function means the wrapped function/class. Will update to clarify this.

return obj.__json_encode__()
if isinstance(obj.trace_symbol, property):
# commonly made mistake when users forget to call the traced function/class.
warnings.warn(f'The symbol of {obj} is found to be a property. Did you forget to create the instance with ``xx(...)``?')
Copy link
Contributor

Choose a reason for hiding this comment

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

this is only a warning, not an error?

Choose a reason for hiding this comment

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

We can't be sure. In rare cases, they might actually want this as expected behavior.

def __enter__(self):
# For example, currently the top of stack is [1, 2, 2], and [1, 2, 2, 3] is used,
# the next thing up is [1, 2, 2, 4].
# `reset_uid` to count from zero for "model_wrapper_1_2_2_4"
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the relation between this label and user provided label?

Choose a reason for hiding this comment

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

If user provides a label, it will always override this label.

@QuanluZhang
Copy link
Contributor

@ultmaster when should users use nni.trace, in what cases? do we have example or doc for the usage, not how to use it but when to use it?

@matluster
Copy link

matluster commented Dec 5, 2021

@ultmaster when should users use nni.trace, in what cases? do we have example or doc for the usage, not how to use it but when to use it?

Users should never use nni.trace, except when they are explicitly instructed to use it (e.g., in NAS and compression cases).

I don't know about compression. For NAS, the planned user interfaces that implicitly use nni.trace are:

  1. @basic_unit and @model_wrapper -- already public API.
  2. @serialize and @serialize_cls -- will stay in the docs for another release. Users will get a deprecation warning that such API will soon be replaced by @nni.trace though. The usage of @nni.trace should be exactly same as @serialize and @serialize_cls.

So, how is @nni.trace different from @serialize?

  1. @nni.trace is much more complicated and should be a lot more robust than @serialize.
  2. Thanks to the new serializer (dump and load), we can now ask the user to write evaluator_fn instead of recursively initializing all the objects used in evaluator with @nni.trace. This is not implemented yet, but should be the next work item coming up after this PR gets merged.

@J-shang
Copy link
Contributor

J-shang commented Dec 6, 2021

This PR will be merged, and @QuanluZhang comments could be fixed in the following PR if they are required.

@J-shang J-shang merged commit 443ba8c into microsoft:master Dec 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

callback in multi_trial example
5 participants