[jit] submodule swapping via module interface #28409

wanchaol · 2019-10-22T04:46:14Z

Stack from ghstack:

[jit] submodule swapping via module interface #28409 [jit] submodule swapping via module interface

Summary:
This PR enables submodule swapping via module interface. User could
declare a submodule as an module interface type in the ScriptModule,
during compilation we will record the module interface type in
ModuleInfo of ConcreteModuleType, the JIT type associated will have the
correct ModuleInterfaceType, and CppModule will get the correct module list

Given that we still keep the module interface type in the type system,
the graph is not inlined when we call Module::Attr and it will use
prim::CallMethod to call the method, this allow us to do module swapping
for the ScriptModule that also meet the same module interface type, and
we only allow the module swapping through the module interface
approach.

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: D18284309

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the attribute as module interface type but still keep the value slot as EntityType::MODULE so that JitType, CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags:

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the attribute as module interface type but still keep the value slot as EntityType::MODULE so that JitType, CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 0a5761b Pull Request resolved: #28409

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the attribute as module interface type but still keep the value slot as EntityType::MODULE so that JitType, CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags:

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the attribute as module interface type but still keep the value slot as EntityType::MODULE so that JitType, CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: b814a05 Pull Request resolved: #28409

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the attribute as module interface type but still keep the value slot as EntityType::MODULE so that JitType, CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the attribute as module interface type but still keep the value slot as EntityType::MODULE so that JitType, CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: f507307 Pull Request resolved: #28409

zdevito

This is a good start but there are a number of issues related to the way we infer the correct types for submodules such that I don't think the conversion logic is correct. @suo can you take a look at this as well? It interacts with the concrete_type logic.

aten/src/ATen/core/jit_type.h

torch/csrc/jit/script/module.h

zdevito · 2019-10-23T21:45:50Z

torch/jit/__init__.py

+                # If the attr is an module interface type, it's guranteed that the module is
+                # not inlined in the graph and acting like the attribute way, in this case we
+                # allow swapping a new scripted module in.
+                if _is_module_interface(self._c, attr):


I think all of the reassigning logic should occur in C++. In fact, giving the tests above I believe all the infrastructure is already written in C++. Having to pybind more functionality into python to accomplish this feels wrong, especially the _is_module_interface and isModule functions. The reassignment error should just fallout naturally from the fact that the rhs will not be the correct subtype.

One thing that has to be happened in python is that we are associating every Script::Module with the actual python module in the OrderedModuleDict, the python module associated after module interface swap has to be updated as well.

zdevito · 2019-10-23T21:59:24Z

torch/jit/_recursive.py


    for name, item in nn_module._modules.items():
+        if name in class_annotations:
+            attr_type = torch.jit.annotations.ann_to_type(class_annotations[name])


I don't like that this is re-parsing type annotations. They should be parsed once and saved.

zdevito · 2019-10-23T22:07:32Z

torch/jit/_recursive.py

                cpp_module._register_parameter(name, orig_value, False)
            elif isinstance(orig_value, torch.jit.Attribute):
                cpp_module._register_attribute(name, attr_type, orig_value.value)
+            elif isinstance(orig_value, Module):


This seems wrong. I feel like if this wasn't done before, we are compiling all submodules twice. @suo who understands this logic better.

concreteType will record the "moduleInterface" modules as attributes instead of modules, so concrete_type.get_module_names() will not get that module and it only get compiled here.

zdevito · 2019-10-23T22:11:03Z

torch/jit/_recursive.py

        added_names.add(name)

    for name, item in nn_module._modules.items():
+        if name in class_annotations:


This logic needs to be carefully done to avoid being fragile. First, it should avoid using continue. Because of continue it forgets to add name to added_names, which is probably a bug. The handling of the submodule is also weird. There is the concrete submodule, which needs to be converted into script using the get_or_create_concrete_type, and there is the interface type which needs to be added as an attribute. This seems to skip handling the concrete_type for the actual module being stored inside this submodule. I think this leads to the problem later where all submodules get recursively scripted again.

zdevito · 2019-10-23T22:11:05Z

torch/jit/_recursive.py


            if is_param:
                cpp_module._register_parameter(name, orig_value, False)
            elif isinstance(orig_value, torch.jit.Attribute):


This copypasta makes this PR more confusing. There should only be one call to _register_attribute in this loop, and it should be clear that it is the fallthrough pathway. I.e. previously modules were going through _register_attribute and the original value is (presumably) the already scripted module.

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the attribute as module interface type but still keep the value slot as EntityType::MODULE so that JitType, CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the attribute as module interface type but still keep the value slot as EntityType::MODULE so that JitType, CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 6fbb32c Pull Request resolved: #28409

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the attribute as module interface type but still keep the value slot as EntityType::MODULE so that JitType, CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

wanchaol · 2019-10-29T00:45:13Z

torch/csrc/jit/script/module.h

+  void setattr(const std::string& name, IValue v) {
+    size_t slot = module_object()->type()->getAttributeSlot(name);
+    const TypePtr& expected = module_object()->type()->getAttribute(slot);
+    // TODO: give better error message when subtyping check fails, and python_str


@suo This is where I think we should give better error message when subtyping check fails, and python_str are just mangled names from the same nn.Module, we shouldn't expose the mangled names to the user.

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the attribute as module interface type but still keep the value slot as EntityType::MODULE so that JitType, CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the module interface type in ModuleInfo of ConcreteModuleType, the JIT type associated will have the correct ModuleInterfaceType, and CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 1875b7f Pull Request resolved: #28409

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the module interface type in ModuleInfo of ConcreteModuleType, the JIT type associated will have the correct ModuleInterfaceType, and CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the module interface type in ModuleInfo of ConcreteModuleType, the JIT type associated will have the correct ModuleInterfaceType, and CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 6294a29 Pull Request resolved: #28409

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the module interface type in ModuleInfo of ConcreteModuleType, the JIT type associated will have the correct ModuleInterfaceType, and CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the module interface type in ModuleInfo of ConcreteModuleType, the JIT type associated will have the correct ModuleInterfaceType, and CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D18284309](https://our.internmc.facebook.com/intern/diff/D18284309) [ghstack-poisoned]

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the module interface type in ModuleInfo of ConcreteModuleType, the JIT type associated will have the correct ModuleInterfaceType, and CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 7c8365b Pull Request resolved: #28409

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the module interface type in ModuleInfo of ConcreteModuleType, the JIT type associated will have the correct ModuleInterfaceType, and CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D18284309](https://our.internmc.facebook.com/intern/diff/D18284309) [ghstack-poisoned]

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the module interface type in ModuleInfo of ConcreteModuleType, the JIT type associated will have the correct ModuleInterfaceType, and CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 4968fe1 Pull Request resolved: #28409

suo

Nice, we're close!

suo · 2019-11-04T17:39:33Z

torch/jit/_recursive.py

-            scripted = recursive_script(orig_value)
+            if isinstance(module_type, torch._C.InterfaceType):
+                # use the interface inference rule to compile the module
+                scripted = create_script_module(orig_value, infer_interface_methods_to_compile(module_type, orig_value))


We need to catch the case where orig_value is already a ScriptModule, otherwise we'll try to recompile it. recursive_script() does this check, perhaps we should sink it into create_script_module() instead.

create_script_module() is also used in ScriptMeta.init_then_script, which is causing recursive error if we would like to move it there. I will figure out something that is reasonable.

suo · 2019-11-04T17:39:56Z

torch/csrc/jit/script/init.cpp

      .def("add_function_attribute", &ConcreteModuleType::addFunctionAttribute)
-      .def("add_module", &ConcreteModuleType::addModule)
+      .def("add_module", (void (ConcreteModuleType::*)(std::string, const TypePtr&))&ConcreteModuleType::addModule)
+      .def("add_module", (void (ConcreteModuleType::*)(std::string, std::shared_ptr<ConcreteModuleType>))&ConcreteModuleType::addModule)


If we called it addModuleInterface and avoided overloading here, this code would be a little nicer (see my other comment)

suo · 2019-11-04T17:43:25Z

torch/csrc/jit/script/module.h

    return module_object()->setAttr(name, v);
  }

+  void setattr(const std::string& name, IValue v) {


I think we should prioritize the correctness of the code over reducing merge conflicts, I would prefer to not let duplications get into the code base if it's not absolutely necessary. So let's just fix up set_attribute here.

suo · 2019-11-04T17:46:01Z

torch/csrc/jit/script/concrete_module_type.h

+  // add a submodule to the ConcreteModuleType can either be ConcreteModuleType
+  // that get constructed recursively, or InterfaceType (Module)
  void addModule(std::string name, std::shared_ptr<ConcreteModuleType> meta);
+  void addModule(std::string name, const TypePtr& type);


It might be confusing to call this addModule, since it implies that any Module ClassType would be a valid argument—in reality, only module InterfaceTypes are allowed. Can we call this addModuleInterface, and check in the implementation that type is an InterfaceType?

suo · 2019-11-04T17:52:41Z

test/test_jit_py3.py

                    # type: (Tensor) -> Tensor
                    return 3 * x

+    def test_module_interface_swap(self):


I'd like to see a few more tests here:

What happens if the original submodule satisfying the interface is a ScriptModule?

Tests around the boundary between ClassType interfaces and ModuleType interfaces (what happens when you assign one kind to the other)

Tests that verify that you can only access interface methods on a module (not any other methods that the original submodule attribute happened to define but were not part of the interface)

etc.

Also, some notes on testing structure:

I think this should not be a single method. Each individual behavior you're testing should be a separate test method. That makes it easier to track down problems if only one of the tests fails in the future.

Possibly we can break these tests out into a new test file, see [jit] split test_type_sharing #28037 as an example.

For more tests, like item 2 and 3 are already exist in the #28408, see test from here. I will just move them together to the new test file under test/jit.

Actually for the move of the tests from #28408 to test/jit/test_module_interface, I will do it in a follow up PR as this is getting too big and hard to review together with the previous changes.

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the module interface type in ModuleInfo of ConcreteModuleType, the JIT type associated will have the correct ModuleInterfaceType, and CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D18284309](https://our.internmc.facebook.com/intern/diff/D18284309) [ghstack-poisoned]

suo

shipit!

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the module interface type in ModuleInfo of ConcreteModuleType, the JIT type associated will have the correct ModuleInterfaceType, and CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D18284309](https://our.internmc.facebook.com/intern/diff/D18284309) [ghstack-poisoned]

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the module interface type in ModuleInfo of ConcreteModuleType, the JIT type associated will have the correct ModuleInterfaceType, and CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 3e3371d Pull Request resolved: #28409

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the module interface type in ModuleInfo of ConcreteModuleType, the JIT type associated will have the correct ModuleInterfaceType, and CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D18284309](https://our.internmc.facebook.com/intern/diff/D18284309) [ghstack-poisoned]

Summary: This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the module interface type in ModuleInfo of ConcreteModuleType, the JIT type associated will have the correct ModuleInterfaceType, and CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: f1884fd Pull Request resolved: #28409

Summary: Pull Request resolved: pytorch/pytorch#28409 This PR enables submodule swapping via module interface. User could declare a submodule as an module interface type in the ScriptModule, during compilation we will record the module interface type in ModuleInfo of ConcreteModuleType, the JIT type associated will have the correct ModuleInterfaceType, and CppModule will get the correct module list Given that we still keep the module interface type in the type system, the graph is not inlined when we call Module::Attr and it will use prim::CallMethod to call the method, this allow us to do module swapping for the ScriptModule that also meet the same module interface type, and we only allow the module swapping through the module interface approach. Test Plan: Imported from OSS Reviewed By: driazati Differential Revision: D18284309 fbshipit-source-id: 2cb843e4b75fa3fcd8c6020832a81014dbff4f03

facebook-github-bot · 2019-11-05T21:41:52Z

This pull request has been merged in 9492994.

wanchaol requested a review from apaszke as a code owner October 22, 2019 04:46

This was referenced Oct 22, 2019

[jit] guard against inheritance on torchscript classes #28407

Closed

[jit] introduce module interface declaration #28408

Closed

facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Oct 22, 2019

wanchaol mentioned this pull request Oct 22, 2019

[WIP] module swapping as attribute in recursive API #27236

Closed

wanchaol requested review from suo and zdevito October 22, 2019 20:14

zdevito reviewed Oct 23, 2019

View reviewed changes

wanchaol commented Oct 29, 2019

View reviewed changes

wanchaol requested a review from zdevito October 29, 2019 00:51

suo requested changes Nov 4, 2019

View reviewed changes

wanchaol requested a review from suo November 4, 2019 23:52

suo approved these changes Nov 5, 2019

View reviewed changes

facebook-github-bot closed this in 9492994 Nov 5, 2019

facebook-github-bot added the merged label Nov 5, 2019

facebook-github-bot deleted the gh/wanchaol/60/head branch November 9, 2019 15:16

masahi mentioned this pull request Jan 14, 2020

[Relay] Add a PyTorch to Relay Parser apache/tvm#4497

Merged

mruberry added the Merged label Oct 28, 2020

[jit] submodule swapping via module interface #28409

[jit] submodule swapping via module interface #28409

Uh oh!

Conversation

wanchaol commented Oct 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zdevito left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

suo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wanchaol Nov 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

suo left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Nov 5, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

wanchaol commented Oct 22, 2019 •

edited

Loading

wanchaol Nov 4, 2019 •

edited

Loading