-
Notifications
You must be signed in to change notification settings - Fork 10
Remove intelligent control input sliders from scenarios #1583
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
base: master
Are you sure you want to change the base?
Conversation
|
This migration causes minor changes in comparison to beta, see the ii3050v2 NT Leadership scenario in which both removed sliders where set to 95%: I personally don't see how we could mitigate these changes, since there are no other sliders that influence the useful demand of lighting in the buildings sector. Since these changes are generally small, I feel that no extra effort is required. |
|
This pull request has had no activity for 60 days and will be closed in 7 days. Removing the "Stale" label or posting a comment will prevent it from being closed automatically. You can also add the "Pinned" label to ensure it isn't marked as stale in the future. |
8fe5084 to
c74dbab
Compare
|
I've updated the migration file name to be the most recent one. I agree with the conclusion that there's no way to account for the settings in the intelligent light control inputs with I migration. So the migration should just only remove the inputs from the scenarios. |
|
Added @louispt1 as a functional reviewer. |
kndehaan
left a comment
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.
Works as it should.
|
The migration will be altered, as the work for quintel/etmodel#4631 will be taken up as well, which will affect the migration. |
f8f16e2 to
49c4898
Compare
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 migration works well and shows the results that I expect from the pseudo code! The migration needs alterations on two points (this was not wrong about the migration itself, but what I found out after testing that was missing from the pseudo code). Point 1 can already be processed, point 2 I still need to figure out how this should be calculated correctly.
1. To be added to the migration
The old input buildings_useful_demand_for_appliances has been renamed to buildings_appliances_efficiency, so it needs to be checked if this input was set in a scenario and the value subsequently needs to be set for the new input name. Old input should be deleted. In pseudo code:
APPLIANCE_EFFICIENCY_OLD_KEY = 'buildings_useful_demand_for_appliances'
APPLIANCE_EFFICIENCY_NEW_KEY = 'buildings_appliances_efficiency'
if APPLIANCE_EFFICIENCY_OLD_KEY set in scenario:
APPLIANCE_EFFICIENCY_NEW_KEY = APPLIANCE_EFFICIENCY_OLD_KEY
delete APPLIANCE_EFFICIENCY_OLD_KEY
2. Modification in migration
There's something not going right in the use case where both demand and intelligent light inputs were set (elsif old_demand_set && intel_light_set), but that's something that goes wrong in the pseudo code (and therefore in the migration). I will try to figure out how this should be done properly, and will rewrite the pseudo code part so it can subsequently be implemented in the migration.
Alright, pseudo-code from point 1 has already been added to the migration. Any further changes are pending on outcome of point 2. |


This PR solves quintel/etmodel#4490 by removing the intelligent control lighting inputs in the scenarios
Goes with: