Skip to content
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

Possible bug: different comments/formulations but duplicated code in write_net_revenue #115

Closed
cfe316 opened this issue Jan 21, 2022 · 11 comments · Fixed by #610
Closed
Labels
bug Something isn't working

Comments

@cfe316
Copy link
Collaborator

cfe316 commented Jan 21, 2022

In write_net_revenue.jl the same expression is written twice for two different values of the CO2Cap setting:

if setup["CO2Cap"]==1 # Mass-based
    # Cost = sum(sum(emissions of gen y * dual(CO2 constraint[cap]) for z in Z) for cap in setup["NCO2"])
    dfNetRevenue.EmissionsCost[y] += sum(value(EP[:eEmissionsByPlant][y,t]) * inputs["omega"][t] * (-1) * dual(EP[:cCO2Emissions_systemwide][cap])
        for t in 1:T)

elseif setup["CO2Cap"]==2 # Demand + Rate-based
    # Cost = sum(sum(emissions for zone z * dual(CO2 constraint[cap]) for z in Z) for cap in setup["NCO2"])
    dfNetRevenue.EmissionsCost[y] += sum(value(EP[:eEmissionsByPlant][y,t]) * inputs["omega"][t] * (-1) * dual(EP[:cCO2Emissions_systemwide][cap])
        for t in 1:T)

the comments are different - one says "Mass based" and one is "Demand + rate based" so it's suspicious to me that the mathematical expressions are the same. I haven't looked into this in detail, so maybe these two should really be the same mathematically. If so, it could be good to have a note that these are really the same, and even better, combine the two expressions with if setup["CO2Cap"] == 1 || setup["CO2Cap"] == 2.

@cfe316 cfe316 changed the title Different comments/formulations but duplicated code in write_net_revenue Possible bug: different comments/formulations but duplicated code in write_net_revenue Jan 21, 2022
@cfe316
Copy link
Collaborator Author

cfe316 commented Feb 2, 2022

This may be related to lines in load_co2_cap.jl where the expression is the same but the comments are different.

  if setup["ParameterScale"] ==1
      inputs_co2["dfMaxCO2Rate"] = Matrix{Float64}(inputs_co2["dfCO2Cap"][:,first_col:last_col])
      # when scaled, the constraint unit is kton, thus the emission rate should be in kton/GWh = ton/MWh
  else
      inputs_co2["dfMaxCO2Rate"] = Matrix{Float64}(inputs_co2["dfCO2Cap"][:,first_col:last_col])
      # when not scaled, the constraint unit is ton/MWh
  end

@sambuddhac
Copy link
Collaborator

Thanks @cfe316 .... Taking a look and fixing shortly.

@cfe316
Copy link
Collaborator Author

cfe316 commented Feb 2, 2022

I don't know if somehow the math works out so that it doesn't need to be scaled. As it's written it's confusing. If it's true that it doesn't need to be scaled then the two lines should probably just be combined; no need to have two copies.

@cfe316
Copy link
Collaborator Author

cfe316 commented Mar 14, 2022

Even with the recent vectorizations, this issue stands. Are CO2 being accounted for correctly?

@xuqingyu
Copy link
Collaborator

They can be combined; but they will disappear if my new carbon cap formulation is added in.

@xuqingyu
Copy link
Collaborator

This will be fixed in
#216

@sambuddhac
Copy link
Collaborator

@xuqingyu , is this the PR that you once retracted? Or, is the fix merged already?

@sambuddhac
Copy link
Collaborator

Because I don't see the duplication any more, it now appears under if setup["CO2Cap"] >=1

@sambuddhac sambuddhac added the bug Something isn't working label Jan 8, 2024
@sambuddhac
Copy link
Collaborator

This seems resolved?

@sambuddhac
Copy link
Collaborator

Reviewing and merging PR #610 will close this issue

@sambuddhac
Copy link
Collaborator

Merge of PR #610 addresses this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants