-
Notifications
You must be signed in to change notification settings - Fork 167
feat: Joiners.contain() and Joiners.containedIn() #1999
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
base: main
Are you sure you want to change the base?
Conversation
|
Implements part of #8 |
| // (<A, B> becomes <B, A>.) | ||
| // TODO Does the requiresRandomAccess check make sense? | ||
| // Shouldn't a right bridge always flip, even if there is no left bridge? | ||
| // TODO For neighborhoods, why create a left bridge index and keep it up to date at all? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not create left index in Neighborhoods. The comment below says that.
| // Note that if creating indexer for a right bridge node, the joiner type has to be flipped. | ||
| // (<A, B> becomes <B, A>.) | ||
| // TODO Does the requiresRandomAccess check make sense? | ||
| // Shouldn't a right bridge always flip, even if there is no left bridge? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. The right bridge doesn't flip for Neighborhoods, because it's queried from the left side. <A, B> never becomes <B, A>, and therefore the index doesn't flip either.
core/src/main/java/ai/timefold/solver/core/impl/bavet/common/index/ContainIndexer.java
Outdated
Show resolved
Hide resolved
In theory, yes. In practice, I've seen hashing overhead kill promising experiments in the past. Have you actually seen this improvement? |
No. We'll need to benchmark it. This PR will allow us to do exactly that. I even suspect that sometimes you'll want to prefer using filtering(a.contains(b)) over joiners.contain(a, b). |
|
The tests succeed, but the conference scheduling PR doesn't TimefoldAI/timefold-quickstarts#972 |
|



Before
After (bigO order of magnitude more scalable)
Refactorings
-- Do not create single/composite KeyRetriever in EqualsIndexer and ComparisonIndexer to unify to one constructor.
-- Extract KeyRetriever creation to IndexerFactory (that class is responsibility for those key tricks)
-- Implement equal joiner flip() as no-op, so all right bridges indexer can just get a flip.
Open questions (added as todo's)
I 'd image this code should feel exactly the same as scoring, but not left side
so always flip basically?
Tasks