Skip to content

Implement mta_execute_model - #306

Open
Luthaf wants to merge 6 commits into
metatensor:metatomic-corefrom
Luthaf:execute-model
Open

Implement mta_execute_model#306
Luthaf wants to merge 6 commits into
metatensor:metatomic-corefrom
Luthaf:execute-model

Conversation

@Luthaf

@Luthaf Luthaf commented Aug 31, 2026

Copy link
Copy Markdown
Member

This should be the last building block for the new C API!

Contributor (creator of pull-request) checklist

  • Tests updated (for new features and bugfixes)?
  • Documentation updated (for new features)?
  • Issue referenced (for PRs that solve an issue)?

Reviewer checklist

  • CHANGELOG updated with public API or any other important changes?

{
let mut cache = ATOMIC_TYPES_CACHE.lock().expect("ATOMIC_TYPES_CACHE lock poisoned");
let atomic_types = &capabilities.atomic_types;
let valid_types = cache.get_or_insert_ref(atomic_types.as_slice(), || {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is atomic_types guaranteed to be sorted? (From the comment, you want the key to be sorted.)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

no, I forgot to sort them in there. I've made this part of the type contract so we don't get it wrong next time.


/// Validate that the inputs to a model are consistent with its capabilities.
///
/// This checks that:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe worth checking also that the device is in the model's supported devices?

for (parameter, gradient) in block.gradients() {
let grad_device = gradient.values().device()?;
let grad_copy = gradient.values().copy(grad_device)?;
let mut dlpack = grad_copy.as_dlpack(device, None, dlpack_version)?;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
let mut dlpack = grad_copy.as_dlpack(device, None, dlpack_version)?;
let mut dlpack = grad_copy.as_dlpack(grad_device, None, dlpack_version)?;

let src_ptr = unsafe { dlpack_to_device_ptr(*tensor) };
let n_elements: i64 = tensor.shape().iter().product();
let elem_size = tensor.dtype().bits as usize / 8;
let num_bytes = n_elements as usize * elem_size;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Worth checking that the tensor is contiguous?

outputs: *mut *mut mts_tensormap_t,
outputs_count: usize,
) -> Result<(), Error> {
let capabilities = model.capabilities()?;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can these be cached? Doing string allocation, JSON conversion, etc at every call might be wasteful.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think we can try to get them at every call for now, and I'll leave a TODO note to measure the overhead of this

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