Skip to content

[6/N] Add update in module #11533

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: gh/cccclai/26/base
Choose a base branch
from
Open

[6/N] Add update in module #11533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
22864ef
[6/N] Add update in module
cccclai Jun 10, 2025
2438cb5
Update on "[6/N] Add update in module"
cccclai Jun 12, 2025
f0c47a6
Update on "[6/N] Add update in module"
cccclai Jun 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion extension/module/module.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,13 @@ runtime::Error Module::set_output(
output_tensor.mutable_data_ptr(), output_tensor.nbytes(), output_index);
}

} // namespace ET_MODULE_NAMESPACE
runtime::Error Module::update(
const std::string& method_name,
runtime::ArrayRef<runtime::Entry> backend_options) {
ET_CHECK_OK_OR_RETURN_ERROR(load_method(method_name));
auto& method = methods_.at(method_name).method;
return method->update(backend_options);
}

Comment on lines +312 to +319
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 exactly my problem. none of the piping being done here is module or method name specific. Because the options being sent down are to the global singleton of backend. It would make sense only if we have a way to set options on an instance of the backend class that is tied to a given method and model.

Also I apologize if this was already discussed in review. It has been a while so likely I forgot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we want to provide backend flexibility to decide whether it's global or per session. There is one single instance of the backend class. It is a singleton and processes all the .pte files (including all methods) even from different processes. We want the backend to have the option as needed.

Additionally, currently users don't interact with backend directly, Users just load the .pte file, construct the method. ET runtime is responsible for passing the information between user and the backends.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we want to provide backend flexibility to decide whether it's global or per session.

I think these would need to be different apis. Otherwise it seems like a huge nightmare for users to reason about what options are local state and what are global. Its not also that easy to debug since ET allows closed source delegates.

Additionally, currently users don't interact with backend directly, Users just load the .pte file, construct the method.

Yeah but since the current backend api is primarily driven towards setting global state, I think it would be more natural for there to be some ET global api for them to call.

Also as an aside these options sort of feel similar to contextGuards in regular torch like inference mode. Maybe we should explore that as an api option here (not sure how portable it is especially since wed want them to be thread local.

} // namespace extension
} // namespace executorch
26 changes: 26 additions & 0 deletions extension/module/module.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,32 @@ class Module {
return set_output("forward", std::move(output_value), output_index);
}

/**
* EXPERIMENTAL: Updates backend options for a specific method.
* Loads the program and method before updating if needed.
*
* @param[in] method_name The name of the method to update.
* @param[in] backend_options The backend options to update the method with.
*
* @returns An Error to indicate success or failure.
*/
ET_EXPERIMENTAL ET_NODISCARD runtime::Error update(
const std::string& method_name,
runtime::ArrayRef<runtime::Entry> backend_options);
Copy link
Contributor

Choose a reason for hiding this comment

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

I dont think we should expose arrayref options in Module. You should just skip to the syntactic sugar vector/map version you have in the next PR.

Unrelated to this PR but similar motivations to why I dont want ArrayRef here. I think its probably a mistake we used EValue in module. I wish we had a different type like extension/OwningEValue or something that let us include Dict and List for unflattened IO and let us have stronger ownership semantics on module IO cc @shoumikhin

Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly Im not sure its valuable to have module return Error vs throwing exception


/**
* EXPERIMENTAL: Updates backend options for the 'forward' method.
* Loads the program and method before updating if needed.
*
* @param[in] backend_options The backend options to update the method with.
*
* @returns An Error to indicate success or failure.
*/
ET_EXPERIMENTAL ET_NODISCARD inline runtime::Error update(
runtime::ArrayRef<runtime::Entry> backend_options) {
return update("forward", backend_options);
}

/**
* Retrieves the EventTracer instance being used by the Module.
* EventTracer is used for tracking and logging events during the execution
Expand Down
42 changes: 42 additions & 0 deletions extension/module/test/module_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,32 @@
#include <executorch/extension/data_loader/file_data_loader.h>
#include <executorch/extension/tensor/tensor.h>
#include <executorch/runtime/core/exec_aten/testing_util/tensor_util.h>
#include <executorch/runtime/backend/backend_options.h>
#include <executorch/runtime/backend/backend_options_map.h>
#include <executorch/runtime/executor/test/stub_backend.h>

using namespace ::executorch::extension;
using namespace ::executorch::runtime;
using executorch::runtime::BackendOptions;
using executorch::runtime::Entry;
using executorch::runtime::IntKey;

class ModuleTest : public ::testing::Test {
protected:
static void SetUpTestSuite() {
model_path_ = std::getenv("ET_MODULE_ADD_PATH");
add_mul_path_ = std::getenv("ET_MODULE_ADD_MUL_PROGRAM_PATH");
add_mul_data_path_ = std::getenv("ET_MODULE_ADD_MUL_DATA_PATH");
stub_model_path_ = std::getenv("ET_MODULE_ADD_MUL_DELEGATED_PATH");

// Register the StubBackend for testing
StubBackend::register_singleton();
}

static inline std::string model_path_;
static inline std::string add_mul_path_;
static inline std::string add_mul_data_path_;
static inline std::string stub_model_path_;
};

TEST_F(ModuleTest, TestLoad) {
Expand Down Expand Up @@ -466,3 +477,34 @@ TEST_F(ModuleTest, TestPTD) {
auto tensor = make_tensor_ptr({2, 2}, {2.f, 3.f, 4.f, 2.f});
ASSERT_EQ(module.forward(tensor).error(), Error::Ok);
}

TEST_F(ModuleTest, TestUpdate) {
Module module(stub_model_path_);

BackendOptionsMap<3> map;
BackendOptions<1> backend_options;
int new_num_threads = 4;
backend_options.set_option(IntKey("NumberOfThreads"), new_num_threads);
map.add("StubBackend", backend_options.view());

// Test update method with specific method name
const auto update_result = module.update("forward", map.entries());
EXPECT_EQ(update_result, Error::Ok);

ASSERT_EQ(StubBackend::singleton().num_threads(), new_num_threads);

}

TEST_F(ModuleTest, TestUpdateNonExistentMethod) {
Module module(stub_model_path_);

BackendOptionsMap<3> map;
BackendOptions<1> backend_options;
int new_num_threads = 4;
backend_options.set_option(IntKey("NumberOfThreads"), new_num_threads);
map.add("StubBackend", backend_options.view());

// Test update method with non-existent method name
const auto update_result = module.update("nonexistent", map.entries());
EXPECT_NE(update_result, Error::Ok);
}
2 changes: 2 additions & 0 deletions extension/module/test/targets.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ def define_common_targets(is_fbcode=False):
"ET_MODULE_ADD_PATH": "$(location fbcode//executorch/test/models:exported_programs[ModuleAdd.pte])",
"ET_MODULE_ADD_MUL_PROGRAM_PATH": "$(location fbcode//executorch/test/models:exported_program_and_data[ModuleAddMul.pte])",
"ET_MODULE_ADD_MUL_DATA_PATH": "$(location fbcode//executorch/test/models:exported_program_and_data[ModuleAddMul.ptd])",
"ET_MODULE_ADD_MUL_DELEGATED_PATH": "$(location fbcode//executorch/test/models:exported_delegated_add_mul[ModuleAddMul.pte])",
}

for aten_mode in get_aten_mode_options():
Expand All @@ -35,6 +36,7 @@ def define_common_targets(is_fbcode=False):
"//executorch/extension/module:module" + aten_suffix,
"//executorch/extension/tensor:tensor" + aten_suffix,
"//executorch/runtime/core/exec_aten/testing_util:tensor_util" + aten_suffix,
"//executorch/runtime/executor/test:stub_backend",
],
env = modules_env,
platforms = [CXX, ANDROID], # Cannot bundle resources on Apple platform.
Expand Down
3 changes: 2 additions & 1 deletion runtime/executor/method.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1513,7 +1513,8 @@ Error Method::experimental_step() {
return step();
}

Error Method::update(executorch::runtime::ArrayRef<executorch::runtime::Entry> backend_option) {
Error Method::update(
executorch::runtime::ArrayRef<executorch::runtime::Entry> backend_option) {
for (const auto& entry : backend_option) {
const char* backend_name = entry.backend_name;
auto backend_options = entry.options;
Expand Down
5 changes: 3 additions & 2 deletions runtime/executor/method.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,13 +241,14 @@ class Method final {
/// DEPRECATED: Use `reset_execution()` instead.
ET_DEPRECATED ET_NODISCARD Error experimental_reset_execution();

/**
/**
* EXPERIMENTAL: Update backend options, which will be dispatched to different backends.
*
* @retval Error::Ok step succeeded
* @retval non-Ok Method update fails
*/
ET_EXPERIMENTAL ET_NODISCARD Error update(executorch::runtime::ArrayRef<executorch::runtime::Entry> backend_option);
ET_EXPERIMENTAL ET_NODISCARD Error update(
executorch::runtime::ArrayRef<executorch::runtime::Entry> backend_option);

/**
* Returns the MethodMeta that corresponds to the calling Method.
Expand Down
Loading
Loading