-
Notifications
You must be signed in to change notification settings - Fork 1
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
Allow for sudden increase in the heat pump awareness #148
Conversation
* Changes allow for a step up in HP awareness as specified in new user inputs.
Before, 100% awareness was assume at the ban announcement date
@@ -277,6 +277,10 @@ def model_heat_pump_installations_at_current_step(model) -> int: | |||
return model.heat_pump_installations_at_current_step | |||
|
|||
|
|||
def model_heat_pump_awareness(model) -> int: | |||
return model.heat_pump_awareness |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the right variable? Should you have the at_current_step
suffix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is referring to the property DomesticHeatingABM.heat_pump_awareness
defined in models.py
which returns households_heat_pump_aware_at_current_step / household_count
, which is what I want.
@@ -150,6 +151,9 @@ def __init__( | |||
self.is_heat_pump_aware = ( | |||
self.heating_system in HEAT_PUMPS or is_heat_pump_aware | |||
) | |||
self.is_heat_pump_aware_after_campaign = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the self.is_heat_pump_aware
logic is: either (already has heat pump) or (is heat pump aware) because by definition, if they have a heat pump, they must be heat pump aware.
I'm not sure if the same logic follows into is heat pump aware after campaign
:
- this variable suggests that this household is aware because of the campaign
- not 100% sure 1) if this variable name actually reflects the variable you're trying to create 2) how this variable will be used later on and whether or not it reflects the variable name
will revisit this comment later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After having read the entire PR, I now understand this variable to be latent variable
that gets activated later on if the campaign date is reached. But i'm still unsure if we need this variable to implement the campaign effect..
def construct_population_awareness_due_to_campaign( | ||
population_heat_pump_awareness: List[bool], | ||
heat_pump_awareness_due_to_campaign: float, | ||
heat_pump_awareness: float, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to check:
heat_pump_awareness_due_to_campaign
: the target awareness we're trying to get to with the campaign
heat_pump_awareness
: the current level of awareness in the absence of the campaign, i.e. baseline awareness?
This function basically tries to select the people who are currently not yet aware, and randomly assign households s.t. after the campaign the total number of 'aware' household = specified target?
If i'm summarising correctly, please could you:
- Make sure that the doc string correctly summarises what the function is trying to do, including the argument definitions, as it's not very clear.
- If my understanding is correct, is
heat_pump_awareness
basically the proportion ofpopulation_heat_pump_awareness
that isTrue
? If so - can we clarify this in the code as it's a bit unclear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes your understanding is correct. I will update the doc strings.
simulation/model.py
Outdated
) -> List[bool]: | ||
"""Construct a list assigning heat pump awareness to each agent in the population after the campaign""" | ||
|
||
# Awareness cannot decrease over time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment and implementation is unclear to me.
The comment #Awareness cannot decrease over time
suggests that these variables are 'changing' over time but isn't heat_pump_awareness_due_to_campaign
and input to the simulation (i.e. the target) so it can't change over time. What changes over time is the awareness _at_time_step
?
I think the code will be a lot clearer if this logic is elsewhere (possibly right at the beginning of the script where simulation inputs are parsed). Options:
- or you output a "warning" to in the log to say
heat_pump_awareness_due_to_campaign
value is overriden withheat_pump_awareness
because value is lower or error or - don't let the simulation run and warn the users of invalid simulation input
I think the soft overriding here is very implicit and unclear and I'm trying to guess what you're doing.
heat_pump_awareness_due_to_campaign = max( | ||
heat_pump_awareness_due_to_campaign, heat_pump_awareness | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to go through all these trouble? Why couldn't we simply set heat_pump_awareness
value to the heat_pump_awareness_due_to_campaign
value and randomly assign households the target awareness value?
My understanding of create_household_agents
function where this function construct_population_awareness_due_to_campaign
is called:
create_and_run_simulation
is the handler function- at the start of simulation, this function will create the
model
(the universe) - will create households, and the households would have been assigned
heat_pump_aware
at the very beginning - then
model.run
will collate all the collector functions, and update the households at each timestep
If you want to model heat pump awareness that will increase over time due to the campaign, then shouldn't all this logic live in collector.py, rather than right at the beginning?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although I see that in agents.py
you're only setting the the attribute is_heat_pump_aware
to this heat_pump_awareness_due_to_campaign
after the campaign date.
Which suggests that this heat_pump_awareness_due_to_campaign
is a 'latent' variable that doesn't get activated until after the campaign date.
In which case:
- I'd recommend naming the variable to something clearer e.g.
will_become_heat_pump_aware due to campaign
or something like that orheat_pump_campaign_target_audience
or something to that effect - And actually explain this in the doc strings because I'm just trying to infer/ guess what you're trying to do and there's no context elsewhere in the code that actually lets me confirm what I think you're trying to do!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And if my guess of what you're trying to implement is correct, I feel like it would be more natural to implement this logic inside agent.py
instead of creating this value right at the beginning. Creates less attributes/ methods.
All this logic could have been in update_heat_pump_awareness
function - just access the heat_pump_aware_at_current_timestep
value you've created, calculate the gap (target - currently aware), and flip households accordingly to achieve the target.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I have defined awareness in model.py
r.t. agents.py
is to ensure that once an agent becomes HP aware, it will always be HP aware. The simulation runs by looping over timesteps for the model horizon, and then at each timestep, loop over all agents (i.e., call functions in agents.py
). If we change information about agents in agents.py
in within this for loop, we need to ensure that information is collected and stored somewhere so that at the next timestep, the agent that became 'HP aware' at the previous timestep, is still remembered to be 'HP aware' at the next timestep. In the current ABM implementation, this is achieved by defining whether a household is heat pump aware is defined outside this for loop in create_household_agents
in model.py
. Agent properties e.g., EPC, heating system, HP awareness etc. are defined in create_household_agents
outside the for loop which makes sense to me.
Given that we can access the full population statistics in create_household_agents
(we only see one agent at a time in agents.py
), it makes sense to me to look at who is HP aware, who is not HP aware here, and then define which of the non-aware HP agents will switch during the campaign. Also, given that 'baseline' or initial HP awareness is defined here in the current ABM implementation, it seems natural to me to also define 'future awareness due to campaign' here.
Add heat pump awareness campaign intervention:
heat-pump-awareness-campaign-date
) and the heat pump awareness achieved due to the campaign (heat-pump-awareness-due-to-campaign
).heat-pump-awareness
) even if the user uses a lower value forheat_pump_awareness_due_to_campaign
.heat-pump-awareness
andheat_pump_awareness_due_to_campaign
, respectively. This logic is captured in the functions:construct_population_awareness
andconstruct_population_awareness_due_to_campaign
.Code no longer updates to 100% awareness at the boiler ban announcement date:
heat_pump_awareness_due_to_campaign=1
.