Skip to content

simpler solver_vars improvement#992

Merged
IgnaceBleukx merged 5 commits into
masterfrom
solver_vars2
May 22, 2026
Merged

simpler solver_vars improvement#992
IgnaceBleukx merged 5 commits into
masterfrom
solver_vars2

Conversation

@tias
Copy link
Copy Markdown
Collaborator

@tias tias commented May 21, 2026

Not as effective as 'solver_vars_1d' from #939 , but simpler and compatible with #990

(also includes the dev/time_ortools script)

Not as effective as 'solver_vars_1d', but simpler and compatible with
@tias tias requested a review from IgnaceBleukx May 21, 2026 09:21
@tias tias mentioned this pull request May 21, 2026
@tias tias mentioned this pull request May 21, 2026
23 tasks
Copy link
Copy Markdown
Collaborator

@IgnaceBleukx IgnaceBleukx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we also want to change the actual solver_var implementation of each solver?
There we could also first do the quick check if var.name is in the varmap or not.
I think the first step should at least be to check if it is a _NumVarImpl or not, currently the frist step is to check if it is num, but that is unlikely and slow to check.

We can do it in a different PR tho, this one is good to go in.

res.append(self.solver_var(cpm_var))
elif isinstance(cpm_var, int):
res.append(cpm_var)
elif is_any_list(cpm_var):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also add a fast path for numpy arrays here? Not sure if it is worth it. It would flatten the np array and operate on that

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we rarely store non-1D numpy(ndvar) arrays in constraints, so, don't think its worth special casing and the return type has to be plain list

@tias
Copy link
Copy Markdown
Collaborator Author

tias commented May 21, 2026

Do we also want to change the actual solver_var implementation of each solver? There we could also first do the quick check if var.name is in the varmap or not. I think the first step should at least be to check if it is a _NumVarImpl or not, currently the frist step is to check if it is num, but that is unlikely and slow to check.

We can do it in a different PR tho, this one is good to go in.

that was the original #938 remark indeed : )

Better in a separate PR so this can already go in...?

@IgnaceBleukx IgnaceBleukx merged commit c761d1f into master May 22, 2026
12 checks passed
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

Successfully merging this pull request may close these issues.

2 participants