Skip to content

feat: add generic model loading#395

Merged
JakubGonera merged 2 commits intomainfrom
@jakubgonera/generic-load
Jun 13, 2025
Merged

feat: add generic model loading#395
JakubGonera merged 2 commits intomainfrom
@jakubgonera/generic-load

Conversation

@JakubGonera
Copy link
Copy Markdown
Contributor

Description

In RnExecutorchInstaller we can use loadModel<T> to install to JSI a method that will load our model without writing the same code for each module. The problem is that this method expects that each module needs exactly a string and a CallInvoker in the constructor, which restricts usage of the method for modules which would have multiple sources or other arguments (e.g. OCR or encoder/decoder modules). At the same time, even if modules have different input on load, the code would look nearly identical, so a generic solution is needed for this case.

This PR allows for generic loading of modules if the type info about the constructor is supplied by REGISTER_CONSTRUCTOR macro. This allows for a generic conversion of arguments from JSI, similar to ModelHostObject. The machinery required for this (ConstructorHelpers.h) is admittedly more complicated than in the case of ModelHostObject due to the fact that we need to specify the types ourselves (we cannot grab method pointers to constructors to infer the types), and due to having CallInvoker as the last argument.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update (improves or adds clarity to existing documentation)

Tested on

  • iOS
  • Android

Related issues

#255

Checklist

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly
  • My changes generate no new warnings

@JakubGonera JakubGonera merged commit 604b90f into main Jun 13, 2025
2 checks passed
@JakubGonera JakubGonera deleted the @jakubgonera/generic-load branch June 13, 2025 12:03
@JakubGonera JakubGonera mentioned this pull request Jun 13, 2025
3 tasks
mkopcins pushed a commit that referenced this pull request Oct 15, 2025
## Description

In `RnExecutorchInstaller` we can use `loadModel<T>` to install to JSI a
method that will load our model without writing the same code for each
module. The problem is that this method expects that each module needs
exactly a string and a CallInvoker in the constructor, which restricts
usage of the method for modules which would have multiple sources or
other arguments (e.g. OCR or encoder/decoder modules). At the same time,
even if modules have different input on load, the code would look nearly
identical, so a generic solution is needed for this case.

This PR allows for generic loading of modules if the type info about the
constructor is supplied by `REGISTER_CONSTRUCTOR` macro. This allows for
a generic conversion of arguments from JSI, similar to
`ModelHostObject`. The machinery required for this
(`ConstructorHelpers.h`) is admittedly more complicated than in the case
of `ModelHostObject` due to the fact that we need to specify the types
ourselves (we cannot grab method pointers to constructors to infer the
types), and due to having `CallInvoker` as the last argument.

### Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] Documentation update (improves or adds clarity to existing
documentation)

### Tested on

- [x] iOS
- [x] Android

### Related issues

#255 

### Checklist

- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have updated the documentation accordingly
- [x] My changes generate no new warnings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants