Skip to content
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

upstreaming #122

Open
danieleades opened this issue Sep 1, 2024 · 3 comments
Open

upstreaming #122

danieleades opened this issue Sep 1, 2024 · 3 comments

Comments

@danieleades
Copy link
Contributor

I notice that this library contains a copy-and-paste of my arithmetic coding library. I'm wondering what changes you had to make that made that necessary and whether it's possible to upstream those changes into my library so that you can use it directly without duplication

@ac-freeman
Copy link
Owner

Hey there! My changes were done in this fork. I believe the main thing was that I moved ownership of the writer reference out of the Encoder. For my purposes, this made things way easier to manage, as I use fresh arithmetic coding operations at periodic intervals when writing to the same output. I believe my changes also did something to make it easier to swap models in and out (maybe by making the model field public).

It was a bit of a quick-and-dirty solution to get things working for a deadline I had at the time. I'm happy to work towards upstreaming in the coming weeks if you think those types of changes fit with your design philosophy.

@danieleades
Copy link
Contributor Author

I'm happy to work towards upstreaming in the coming weeks if you think those types of changes fit with your design philosophy.

i'm not averse to the idea if it unlocks some benefit. I've not dug deeply into your codebase, but it sounds like you're swapping out different models based on the input and doing that by tracking state manually, outside the encoder.

The idiomatic way to use the library would be for this state to live inside the model. You would implement a structure which was a facade over the different models you need, which switched between them based on the input. If you need to encode additional metadata to determine when to switch models you would modify your input symbols to also include the metadata.

I believe this would be entirely equivalent, but without exposing the internals of the encoder/decoder. Would also provide much better separation of concerns.

Does that make any sense?

so right now you have an iterator of 'symbols' and you switch models based on 'events'.

That would be equivalent to having something like-

enum Input {
  Event(Event),
  Symbol(Symbol)
}

struct MyModel {
  // ...
}

impl Model for MyModel {
  type Symbol = Input;
  // ....
}

Where MyModel tracks the state and contains the logic needed to choose the correct context

@danieleades
Copy link
Contributor Author

@ac-freeman I'd be keen to support such an effort if you're interested? I'm curious to see whether this genuinely does result in a nicer architecture or if your use-case is genuinely better supported by allowing the model to be swapped by the calling code.

Could you walk me through the relationship between 'symbols' and 'events' in your algorithm?

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

No branches or pull requests

2 participants