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

Any reason for not having a common interface between feedbacks and observers? #2677

Open
CowBoy4mH3LL opened this issue Nov 10, 2024 · 7 comments

Comments

@CowBoy4mH3LL
Copy link
Contributor

I see 4 types of relations between the feedbacks and observers

  1. Feedbacks that accept any observers (e.g. Diff)
  2. Feedbacks that accept observers of specific traits (e.g. Map, withHash, etc.)
  3. Feedbacks that accept observers of specific types (e.g. list, std, etc.)
  4. Feedbacks that do not accept any observers (e.g. const, crash, etc.)

That is to say that there is no solid interface between the feedbacks and the observers, also as per the paper.

Have you thought about standardizing this interface, or are there any super dauting challenges?
The way I see it, the only visible difference amongst Observers is their output.
So is it not a better option to have a standardized interface that outputs a generic instead?

Maybe I am not seeing something you guys have seen before. If so, would love a conversation on it.

@domenukk
Copy link
Member

How would that look in practice?
The way I see it:
A MapFeedback goes through a map. It cannot really accept any other observers?
Likewise, a ListFeedback goes a list, there's not really any other thing it can work with.

The only question might be if the strict separation of observers and feedbacks makes sense since they are closely related.
The concept here is that observer results can be sent over the wire and feedbacks then reduce those observations to a boolean decision.

Would love to hear your suggestions :)

@CowBoy4mH3LL
Copy link
Contributor Author

The thing is, one feedback can work with multiple observers, so how about add a method like get_observations() -> <some associated type>? This would make more sense even when sending over the wire.
For feedbacks that do require specific types like map or list or time, they will get what they need over get_observations. They do not need to access generic types.
For feedbacks that can work over generics this would be great boost, e.g. for diff we do not need another input function to do the actual diffing.

As far as the feedback and observers design choice, I totally agree with it. Feedbacks are data processors and observers are gatherers. Clearly it is a many-to-many relationship and therefore having them as separate entities is absolutely necessary. Having said that, I think, the common get_observations type interface is also necessary to support this design paradigm.

What say?

@domenukk
Copy link
Member

I don't really get where/how the example would fit in, sorry(?)

@CowBoy4mH3LL
Copy link
Contributor Author

Say we change the interface to observers

pub trait Observer{
.... <already existing stuff>
type obs_to_return; // could be things like vectors, lists, time, ...
fn get_observations() -> Self::obs_to_return;
}

Now feedbacks can simply call get_observations and maybe constraint based on the obs_to_return -- e.g. A feedback that operates on list items can restrict Self::Observer::obs_to_return: Iterator -- and we can even extrapolate obs_to_return to ObserversTuple.

Off the top of my head, I do not see an issue with this.
Advantage this offers is obvious -- feedbacks and observers can be developed separately without worrying about how the other would look

@domenukk
Copy link
Member

Can't you already just add another trait like trait ListObserver { fn observed_list() -> List } and add that as a trait bound?

@domenukk
Copy link
Member

Like, I'm not saying it isn't a good idea, I just don't fully understand it. If you want/have time, you can do a proof of concept?

@CowBoy4mH3LL
Copy link
Contributor Author

Sorry for the late reply -- its ok -- let me whip up a small POC !!
But in general, the idea is to add a new method to the Observer trait that will return the observation in a defined format.
This will help write feedbacks agnostic of observer types, better.
I'll have the POC done in sometime.

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