Skip to content

Update postprocessor.py#4

Open
mlmaskey wants to merge 1 commit intoWater-Systems-Management-UCM:masterfrom
mlmaskey:patch-2
Open

Update postprocessor.py#4
mlmaskey wants to merge 1 commit intoWater-Systems-Management-UCM:masterfrom
mlmaskey:patch-2

Conversation

@mlmaskey
Copy link

Adding new lines to export more information such as objective function values, the matrix of unit cost, and target demand.

Modified lines: 92-94, 139, 147-151, and 180-182

These new features allow the end-user to investigate the objective function values at each link and time step as well as compare the target demand vs supplies.

Mahesh Maskey August 15, 2020

Adding new lines to export more information such as objective function values, the matrix of unit cost and target demand
Modified lines: 92-94, 139, 147-151, and 180-182
These new features allow the end-user to investigate the objective function values at each link and time step as well as compare the target demand vs supplies.
@nickrsan
Copy link
Member

Hi Mahesh,

I'm going to request that Mustafa review this change as well - It looks safe to me, but I'm less familiar with the postprocessor code and want to make sure we don't have anything weird going on.

I think before merging, I'd also want to resolve the dual definition of ub and unit_cost - they're defined in a conditional on lines 135/136 and then defined again in the new code - We should determine if it's only valid in the conditional, or if we can safely extract that out of the conditional and use it in both spots. Without that, I'd be concerned about weird code maintenance issues in the future.

@nickrsan
Copy link
Member

@msdogan - Please see my comment above - I also sent a request to make you an external collaborator for this organization so I can tag you for any future reviews. Thanks!

@mlmaskey
Copy link
Author

Hi Mahesh,

I'm going to request that Mustafa review this change as well - It looks safe to me, but I'm less familiar with the postprocessor code and want to make sure we don't have anything weird going on.

I think before merging, I'd also want to resolve the dual definition of ub and unit_cost - they're defined in a conditional on lines 135/136 and then defined again in the new code - We should determine if it's only valid in the conditional, or if we can safely extract that out of the conditional and use it in both spots. Without that, I'd be concerned about weird code maintenance issues in the future.

@nickrsan
Yes, I redefined ub and unit_cost because they were inside the condition. In order to accommodate, I did like this intentionally. If you need clarification, let's discuss it among the three of us.
Thank you, @nickrsan

@msdogan
Copy link

msdogan commented Aug 18, 2020

Hi @mlmaskey, I believe objective function value valFobj would be useful in the outputs file but, unit cost valCost and upper bound constraint valTarget are actually inputs not outputs. So, it might be confusing for others. Also, upper bound represents mostly capacity constraints. It represents target demands only for demand areas. So that might further confuse others. I suggest you keep those changes in your local repository for own calculations.

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.

3 participants