Add water_network instance #59
Conversation
tristantc
force-pushed
the
water_network
branch
from
af2e3e2
to
f784ca0
Compare
|
Hi @tristantc . The linter check failed.
|
|
Thanks for the feedback, @ZedongPeng. |
| @@ -0,0 +1,5 @@ | |||
| cont,T | |||
There was a problem hiding this comment.
The CSV file looks small. Maybe instead of creating the CSV file, we should define a dictionary in the Python script. This also applies to TU.csv and feed.csv.
There was a problem hiding this comment.
We would like to scale this instance to more cases, so I rather leave all in CSVs
| @@ -3,3 +3,4 @@ | |||
| hda = "hda" | |||
| HDA = "HDA" | |||
| equil = "equil" | |||
| ue = "ue" | |||
There was a problem hiding this comment.
ue stands for unit_exists
gdplib/water_network/README.md
Outdated
|
|
||
| ### Size | ||
|
|
||
| | Component | pwl | quadratic | |
There was a problem hiding this comment.
Chose not to upload the MINLP model because it's not a GDP model, but a GAMS model transformed into pyomo or are you asking about the model size table?
There was a problem hiding this comment.
I incorporated the original model without any approximation.
| @@ -0,0 +1,5 @@ | |||
| cont,T | |||
There was a problem hiding this comment.
We would like to scale this instance to more cases, so I rather leave all in CSVs
| == ue.flow[mt, unit] | ||
| ) | ||
|
|
||
| elif approximation == "piecewise": |
There was a problem hiding this comment.
quadratic2 function fits original function better than quadratic, but not zero at zero flowrate.
Best known objective: $348,340
quadratic $346,672
quadratic2 $349,555
There was a problem hiding this comment.
Either in the documentation or in the README explain what quadratic and quadratic2 are. I would recommend to rename the models to quadratic_zero_origin and quadratic_nonzero_origin
There was a problem hiding this comment.
Does it make sense to hold onto both versions?
BigM and Hull MINLP reformulations work. |
| ue.streams, | ||
| doc="TU streams flowrate", | ||
| domain=pyo.NonNegativeReals, | ||
| # bounds=lambda _,i,k:(TU.loc[unit,'L'],100) # Upper bound for the flow rate from Ruiz and Grossmann (2009) is 100 |
There was a problem hiding this comment.
Why not adding the same bound as the original instance?
There was a problem hiding this comment.
The overall feed flow rate is less than 100.
It provides increased flexibility for multiple cases.
| # Objective function: minimize the total cost of the treatment units | ||
| m.obj = pyo.Objective(expr=sum(m.costTU[k] for k in m.TU), sense=pyo.minimize) | ||
|
|
||
| # init_vars.InitMidpoint().apply_to(m) |
There was a problem hiding this comment.
Do we need to add an argument for the initialization? Or just remove it?
There was a problem hiding this comment.
I opted not to use it, saving it in case of uninitialized variables.
I can remove the comment.
There was a problem hiding this comment.
I would leave the comment, I didn't know one could do this, but I would also add an extra comment before explaining what this line does
This pull request incorporates the
water_networkinstance.Please review the changes and provide feedback. Thank you!