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

Splits the input shares to 1/x'th of the edges #59

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

grdw
Copy link

@grdw grdw commented May 7, 2018

Fixes: #58

@grdw grdw requested a review from antw May 7, 2018 15:17
@grdw grdw assigned antw May 7, 2018
@codecov
Copy link

codecov bot commented May 7, 2018

Codecov Report

Merging #59 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #59      +/-   ##
==========================================
+ Coverage   93.62%   93.64%   +0.01%     
==========================================
  Files          27       27              
  Lines         659      661       +2     
==========================================
+ Hits          617      619       +2     
  Misses         42       42
Impacted Files Coverage Δ
lib/refinery/slot.rb 95.12% <100%> (+0.25%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9272d50...1932794. Read the comment docs.

@antw
Copy link
Contributor

antw commented May 8, 2018

This doesn't feel like the right approach to me. Refinery is all about being given information and it filling in the blanks based on that information. If it doesn't have enough data to give a definitely correct answer, it throws an error at you saying that it needs more. This PR would allow Refinery to create entirely fictitious data to cover up the fact that not enough is being provided.

To use the example from #58:

graph

When the demand of A,B,C = 0. The input slot ratio of D (input slot A-D, B-D, C-D) should be able to calculate

The reason why Refinery does not do this is that the default graph may have no energy flowing through these nodes, but the user may change inputs in ETModel which does cause energy to flow. If Refinery creates fake shares for these slots, then so too will the flow of energy through these nodes would be wrong.

To use the concrete example from ETLocal: the default shares of coal, crude oil, and network gas into "industry other metals burner" is 55%, 2%, and 43%. If we were to allow Refinery to fake this, the shares of these carriers would be 33.3%, 33.3%, 33.3% which is incorrect.


I agree the SystemStackError is unacceptable, but I think the fix for missing slot shares lies in ETLocal (or with the user) and not Refinery.

@grdw grdw closed this May 8, 2018
@grdw
Copy link
Author

grdw commented May 8, 2018

I agree the SystemStackError is unacceptable, but I think the fix for missing slot shares lies in ETLocal (or with the user) and not Refinery.

Fair enough. I think that we should probably disallow setting certain demands to 0.

@michieldenhaan
Copy link

michieldenhaan commented May 8, 2018

Maybe Refinery is not the right place to solve this, but how about ETEngine? This is not specifically an ETLocal problem, national datasets have this problem as well.

Take for example transport_road_mixer_diesel. This node does not have ~input.diesel = , ~input.bio_diesel etc. In most cases these input attributes are not required, because as long as one of the input carriers is non-zero, ETEngine (or some other module doing the calculation) is able to calculate the input slot shares, simply by calculating input slot share of diesel = diesel coming into the mixer / (diesel + bio_diesel + .. + .. coming into the mixer). However, when all input carriers are zero, we get a stack overflow because the denominator becomes zero.

To avoid overflow errors, someone has added input. attributes to some (but not all) nodes where this problem can occur. See for example transport_road_mixer_lng, which is conceptually identical to transport_road_mixer_diesel but with input attributes (because some countries don't use LNG for road -> stack overflow). These input attributes serve no purpose except when transport_road_mixer_lng demand is zero. At the same time, these input statements lead to a lot of overhead: for each national dataset csv files with child shares have to be added, ETDataset has to generate these csv's etc. And now we have to incorporate this overhead into ETLocal as well.

I think it is highly desirable to add some rule to ETEngine (or some other module) that says: calculate slot shares the usual way, except when the demand of the node is zero (i.e. there is no energy flowing through this node), then just set some arbitrary numbers summing to 1 (much like a flexible in a share group).

@michieldenhaan michieldenhaan reopened this May 8, 2018
@ChaelKruip
Copy link

I think it is highly desirable to add some rule to ETEngine (or some other module) that says: calculate slot shares the usual way, except when the demand of the node is zero (i.e. there is no energy flowing through this node), then just set some arbitrary numbers summing to 1 (much like a flexible in a share group).

I think this is a useful discussion but I respectfully disagree that it is desirable to set "arbitrary numbers" to set input shares. My main reason is that it is not transparent to the user (or energy researcher) that a decision is being made which might impact the energy system of the region. Perhaps it is "overhead", but I'd much rather have some explicit input's set in the node files than a 'silent' choice being made in the background by either refinery or etengine.

@grdw
Copy link
Author

grdw commented May 8, 2018

In my honest opinion it's still a bug that a demand of 0 can trigger a SystemStackError. How that get's resolved .... I wouldn't know. Perhaps @antw is willing to take a good guess.

How I've solved it in the case of ETLocal is to give all of the 'demand' inputs a default value higher than 0 (read: 1). This temporarily resolves the issue, until somebody reduces it explicitly to 0.

@michieldenhaan
Copy link

@ChaelKruip I'm not sure if you understand me correctly. What I propose is just as 'arbitrary' as a flexible share (which we use a lot) and there should not be any impact on the energy system because as soon as the demand of the node does become positive, ETEngine can calculate the input shares the usual way.

@ChaelKruip
Copy link

What I propose is just as 'arbitrary' as a flexible share (which we use a lot) and there should not be any impact on the energy system because as soon as the demand of the node does become positive, ETEngine can calculate the input shares the usual way.

Thanks for clarifying!

I was thinking along the lines of the flexible shares which determine a slider-group. For instance, the technologies delivering heat to the local district heating:
image
If there is no district heating (in the present graph of a region), no energy flows through this share-group. One of the shares is chosen to be the 'flexible' one and defaults to 1.0 (as the rest are zero).

Now, if the user choses to use district heating, all of sudden there is energy flowing through the node whose share is 1.0! This is an arbitrary node, as we could have chosen another one to be the flexible one in the group.

Why I don't think this is bad is because it is transparent in two ways:

  • The user sees that one slider is 100% in the front-end
  • The modeller/researcher sees that this slider is the flexible one in the back-end

I might be still not understanding what we are talking about here (sorry about that!) but I think it would be unfavourable if we introduce a similar situation with inputs (i.e., where the flow of energy in the future year would be determined by an arbitrary choice) but without the transparency.

If the situation is like you say and "ETEngine can calculate the input shares the usual way" as soon as there is positive demand in all cases, I am less strongly against the idea 😜

@michieldenhaan
Copy link

For the relevant nodes in industry such a situation cannot occur, because if an industry sector is zero in the present you cannot make it positive in the future. For the transport mixers, I guess that if there are no LNG vehicles in the present and you decide to use them in the future, their initial LNG mix will be determined by the arbitrary shares chosen by ETEngine. I.e. these sliders:

monosnap 2018-05-08 16-44-43

In the front-end, the user can see this. In the back-end, this will indeed be difficult. That's not so nice, I agree with that. However, in the current situation it is also very difficult for the modeller/researcher to find out which input is 'flexible'. (1) Open the relevant node .ad file (2) Look for~input.lng(.... , ....) (3) open a csv in ETSource to see the numbers (4) open ETDataset to see how these numbers are calculated (5) look for one cell in Excel that is defined as 1 - SUM() -> that one is flexible. So the flexible input share is currently not explicitly defined as flexible either..

Would be nice if we could specify somewhere which input slot is treated as flexible. In that case, the rule is something like: calculate slot shares the usual way, except when the demand of the node is zero, then set the input slot defined as flexible to 1.

antw added a commit that referenced this pull request May 8, 2018
Avoids infinitely recursing through sibling slots when all demands are
zero.

Ref #58
Ref #59
@antw antw removed their assignment Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants