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

Code replication in module oracle.py #8

Open
tbazin opened this issue Jun 23, 2015 · 5 comments
Open

Code replication in module oracle.py #8

tbazin opened this issue Jun 23, 2015 · 5 comments
Assignees

Comments

@tbazin
Copy link
Collaborator

tbazin commented Jun 23, 2015

The codes for classes FactorOracle, MO and VMO are all very close and share many identical lines, making it hard to uniformly update it.
This could probably be fixed by refactoring the code.

@wangsix
Copy link
Owner

wangsix commented Jun 23, 2015

My original intent was to have the basic FactorOracle as the parent class. FO and MO inherited FactOracle. For VMO, it`s a variant of MO that is still under experiment. I agree that right now it is pretty difficult to update everything consistently, so please let me know your suggestions.

@tbazin
Copy link
Collaborator Author

tbazin commented Jun 23, 2015

I'm currently looking at it to see to which extent we can take advantage of the inheritance mechanism to simplify everything. Might not be super easy, but it'll save us some hassle in the future!
I'll keep You update on that.

@tbazin
Copy link
Collaborator Author

tbazin commented Jun 23, 2015

For starters, I think the function add_state should be modified to be uniform across all implementations.
What really differs between the different versions is the way distances between data are computed and how new clusters are created, ie. the symbolization.

With this done, -- if I'm not mistaken -- the processing can be done on an abstract level, regardless of the underlying data's type.

@tbazin tbazin self-assigned this Jun 23, 2015
@wangsix
Copy link
Owner

wangsix commented Jun 23, 2015

What do you mean by modifying them to be uniform? Thanks!

@tbazin
Copy link
Collaborator Author

tbazin commented Jun 23, 2015

Have it become an explicit function within the base class FactorOracle, with an implementation shared by all subclasses (this would find the lrs, the transitions, etc. based on purely comparing the symbols) which would make calls to some functions like symbolize (or whatever).
These auxiliary functions would themselves be sub-classed, with respect to the various implementations. They would for instance turn a concrete data-chunk for the VMO into a symbol for the Factor Oracle and add the corresponding symbol to the oracle's states.

Is it clear?

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

No branches or pull requests

2 participants