[Executorch] Make module constructors uniform across#15709
[Executorch] Make module constructors uniform across#15709kimishpatel wants to merge 1 commit intogh/kimishpatel/209/basefrom
Conversation
Existing constructors dont compose well such that if you want data loader or data files constructor then you cannot get to override memory allocator. Fix that. Differential Revision: [D86120037](https://our.internmc.facebook.com/intern/diff/D86120037/) [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/15709
Note: Links to docs will display an error until the docs builds have been completed. ❌ 14 New FailuresAs of commit 799c34b with merge base aba44fd ( NEW FAILURES - The following jobs have failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
This PR needs a
|
| explicit Module( | ||
| const std::string& file_path, | ||
| const LoadMode load_mode = LoadMode::File, | ||
| std::unique_ptr<runtime::MemoryAllocator> memory_allocator = nullptr, |
There was a problem hiding this comment.
nit: method_allocator
There was a problem hiding this comment.
oh I see. This is supposed to be called method_allocator?
| const LoadMode load_mode = LoadMode::File, | ||
| std::unique_ptr<runtime::MemoryAllocator> memory_allocator = nullptr, | ||
| std::unique_ptr<runtime::MemoryAllocator> temp_allocator = nullptr, | ||
| std::unique_ptr<runtime::EventTracer> event_tracer = nullptr); |
There was a problem hiding this comment.
This is technically a bc breaking change
There was a problem hiding this comment.
That is true. Is this part of core API btw given that it is in extension?
I do think we should unify the interface though.
|
Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as |
|
Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as |
2 participants
Stack from ghstack (oldest at bottom):
Existing constructors dont compose well such that if you want data loader or data files constructor then you cannot get to override memory allocator.
Fix that.
Differential Revision: D86120037