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

Consolidate all volume and intersection functions #10

Open
mboratko opened this issue Nov 16, 2020 · 5 comments · Fixed by #41
Open

Consolidate all volume and intersection functions #10

mboratko opened this issue Nov 16, 2020 · 5 comments · Fixed by #41
Assignees
Labels
enhancement New feature or request

Comments

@mboratko
Copy link
Collaborator

mboratko commented Nov 16, 2020

I think it would make sense to consolidate all volume (and intersection) functions into a single one, where if the variance is zero we revert to the "hard" versions of these functions.

@mboratko mboratko changed the title Collapse all intersection / volume functions Consolidate all volume and intersection functions Nov 16, 2020
@dhruvdcoder
Copy link
Collaborator

dhruvdcoder commented Nov 16, 2020

Do we want to unify all intersections and Volumes under single function? Or do we have 'if conditions' inside each one to revert to hard version based on beta value?

@mboratko
Copy link
Collaborator Author

mboratko commented Nov 18, 2020

I meant the latter.

Note that this is a situation where the functional interface and class-based interface can (and should) actually be different. For example, the functional interface will include an "if" statement based on the variance (for example), but a class will assign the correct branch during initialization.

(Maybe this is a premature optimization though.)

@mboratko mboratko reopened this Nov 18, 2020
@dhruvdcoder
Copy link
Collaborator

We can leave this open and come back to it when we have a better idea about whether the optimization is needed or not.

@dhruvdcoder
Copy link
Collaborator

Add an example in the usage example notebooks.

@trangtran72
Copy link
Collaborator

Trang will address this in the upcoming PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants