WIP: Add DataFrames as a weak dependency #3441
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #3441 +/- ##
=======================================
Coverage 98.01% 98.01%
=======================================
Files 36 38 +2
Lines 5039 5050 +11
=======================================
+ Hits 4939 4950 +11
Misses 100 100
☔ View full report in Codecov by Sentry. |
| function test_dimension_data_variable() | ||
| model = Model() | ||
| @variable(model, x[i = 2:4, j = 1:2], container = DataFrame) | ||
| @variable(model, y[i = 2:4, j = 1:2; isodd(i+j)], container = DataFrame) |
There was a problem hiding this comment.
The more I think about this, the less I like it. I prefer the explicit "add a new column to an existing dataframe" than the "construct a dataframe from these indices." The latter approach is just going to encourage the same nested loops as the GAMS example.
|
Ideally, we should parse the condition of the
If it involves a subset of the indices then we should filter with this condition earlier and not in the most nested level of the for loops. In some cases, it might be better to do the nested for loops in an order that is different from the order of the indices used by the user but that can be left as future work since I don't think this is easy to find that. But detecting the two things I mentioned above and improving the iteration shouldn't be too complicated and it would solve the issue in the GAMS example without the user to do anything. The |
This is getting a bit too magical. It also assumes that constructing the index sets and a dictionary solves the performance problem. I don't think we need to "fix" the GAMS example. We need to encourage people to rethink how they view models. Nested for-loops are not the best way to conceptualize the IJKLM model. |
It makes the complexity depends linearly on the number of tuples satisfying the condition. It might be surprising to the user that we build the Following this PR, how would the user solve the IJKLM issue ? |
They wouldn't. That's why I said I don't really like this PR. |
Closes #3438
I started working on this, but I don't know whether I like it or not.
I think I prefer the approach of explicitly constructing a data frame, and then adding a new column which is a vector of variables. It's more explicit and gives the user control over what the call the new column. This current approach would likely also be used naively to construct sparse sets which doesn't fix the loop problem.