Skip to content

engine.wrap Ops #283

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.

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

engine.wrap Ops #283

wants to merge 3 commits into from

Conversation

gselzer
Copy link
Member

@gselzer gselzer commented Feb 11, 2025

This PR, based on a branch that was designed simply to streamline the conversion of RandomAccessibleIntervals via wrapping, has been taken over to add a new feature to Op conversion. Namely:

Can we convert Computers without that annoying copy at the end??

Background

Suppose you want to convert a Computer<I, O> into a Computer<A, B>. Currently, you need (at a minimum):

  • An engine.convert Op that transforms your A object into an I object.
  • An engine.convert Op that transforms your B object into an O object.
  • An engine.copy Op that puts the data from an O object into your B object or an engine.convert Op that transforms your O object into a B object and an engine.copy Op to put the data from the intermediary B object into the one you gave it.

In many cases, however, those engine.convert Ops will be most performant when wrapping the object. If that wrapper is read/write (as is the case for ndarray -> RAI wrapping via imglyb, for example), when the Computer<I, O> backing your Computer<A, B> Op is called, the changes will be written through into the original B object, and the later engine.convert/engine.copy is unnecessary work.

A new Op name: engine.wrap

engine.wrap Ops are object conversions via a read/write wrapper. While not enforced, all engine.wrap Ops should be aliased engine.convert (and likely also convert.X).

When performing conversion, the Op conversion MatchingRoutine will watch for the usage of engine.wrap Ops. If used for a preallocated output, the matching routine will avoid matching the output engine.convert/engine.copy Ops.

  • In practice, this is a very simple change, as it is equivalent to the existing behavior that watches out for the engine.identity Op

TODO:

  • The proof of correctness here will be with inter-ComplexType image conversion, and as of right now the tests fail 😦. These tests must be fixed.
  • Clean up PR (commits + code)
  • Additional tests?
  • Bump SciJava Ops Engine minor version, signifying new behavior.

@gselzer gselzer requested a review from hinerm February 11, 2025 00:48
@gselzer gselzer self-assigned this Feb 11, 2025
@imagesc-bot
Copy link

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/fiji-friends-weekly-dev-update-thread/103718/60

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants