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

Fixes/#220 #237

Open
wants to merge 47 commits into
base: release/v0.4.0
Choose a base branch
from
Open

Fixes/#220 #237

wants to merge 47 commits into from

Conversation

eosram
Copy link

@eosram eosram commented Mar 19, 2018

The implementation of the timeseries generator script gave often rise to questions. The script intended by me as a quick workaround is not very human readable anymore. With #220 as a starting point I tried to reimplement it in python.

Copy link
Member

@IlkaCu IlkaCu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering if this conflicts with the changes @MarlonSchlemminger implemented in #233.

@MarlonSchlemminger
Copy link
Contributor

Yeah, #233 and #227 as well would need to be included in the python script. Just checked on my local database and at least for p_max_pu the bug is gone after 233.

@eosram
Copy link
Author

eosram commented Mar 20, 2018

Thank you for your feedback @MarlonSchlemminger , @IlkaCu ! If I got this right p_max_pu have to be set based on ren_feedin_by_gen_id for German renewable sources and ren_feedin_foreign for all other sources. Offshore for foreign countries is handled differently and is based on ego_renewable_feedin, ego_neighbours_offshore_point. Is this correct @MarlonSchlemminger ? Do I miss something? If so, I guess, that's where I could start.

Martin added 7 commits April 1, 2018 21:25
@eosram
Copy link
Author

eosram commented Apr 2, 2018

There might be a problem in the current dev.

DROP materialized view IF EXISTS model_draft.ren_feedin_by_gen_id;
CREATE materialized view model_draft.ren_feedin_by_gen_id
AS
SELECT
gen.generator_id, feedin.feedin
FROM
(SELECT
aggr_id AS generator_id,
w_id,
power_class,
source
FROM
model_draft.ego_supply_pf_generator_single
WHERE source IN (12, 13, 17)
AND scn_name = 'Status Quo'
GROUP BY aggr_id, w_id, power_class, source) AS gen,
(SELECT
w_id,
power_class,
CASE
WHEN source LIKE '%%solar%%' THEN 12
WHEN source LIKE '%%wind_onshore%%' THEN 13
WHEN source LIKE '%%wind_offshore%%' THEN 17
END AS source,
feedin
FROM model_draft.ego_renewable_feedin) AS feedin
WHERE gen.source = feedin.source
AND gen.w_id = feedin.w_id
AND gen.power_class = feedin.power_class;

The query will return multiple duplicate generator ids with corresponding, but different feedins based on the different power_class, w_id (?).

-- set p_max_pu as timeseries from ego_renewable_feedin
UPDATE model_draft.ego_grid_pf_hv_generator_pq_set A
SET p_max_pu = feedin.feedin
FROM model_draft.ren_feedin_by_gen_id AS feedin
WHERE A.generator_id = feedin.generator_id;

The update is based on generator_id alone which will assign one of the multiple corresponding feedins provided by the previously constructed view, if I'm not mistaken.

This is related to #245. If you think, it's best, I will reopen that pull reqest!

I tried to handle it in f4765af afccca9. With regard to this pull request there is unfortunately still some/a lot work to do and I'm struggling with the different ids. Maybe I can ask you about it @IlkaCu

@MarlonSchlemminger
Copy link
Contributor

In my local view, there is only one entry per generator_id. There are some rows without a generator_id where I don't know why they exist, but they shouldn't have any effect because they will not be used during the assignment of the feedins. I can't test it on the oedb, because the view doesn't exist there. Where did you get the problem, on your local database? Did you run the new version of ego_dp_powerflow_assignment_generator.sql before?

@eosram
Copy link
Author

eosram commented Apr 3, 2018

Deleted my previous comment to avoid further confusion. Thanks @MarlonSchlemminger, I assumed the aggr_id would be a unique combination of scn_name, bus, source, but I now see, that it also takes into account the w_id and power_class.

Martin added 4 commits April 9, 2018 22:10
Atm timeseries generator handles discrepancies within the scenario
definition of renpass_gis and powerflow.
Martin added 16 commits April 17, 2018 18:21
Discrepancies in scenario defintion is handled in a separate issue
znes/FlEnS#3
p_set Germany
Timeseries_generator_other_p_set does only write timeseries data to the
field p_set in case of neighbouring countries.
Neighbours table has to be used to determine the bus_id based on
country codes.
@eosram
Copy link
Author

eosram commented Apr 17, 2018

Hmm, does ego_neighbours_offshore_point have the right permissions set? I do not know why, but I'm not able to make a backup of it.

@wolfbunke
Copy link
Member

right, I just changed it ✅

@eosram eosram changed the base branch from dev to release/v0.4.0 April 18, 2018 21:07
@eosram
Copy link
Author

eosram commented Apr 18, 2018

Still missing in this PR to address bug #220 is the assignment of offshore wind feedin data for neighbouring countries, which I can maybe add on Friday at the earliest. Also, and that's maybe a dealbreaker, the PR does rely on an updated ego.io https://github.com/openego/ego.io/tree/fixes/dataprocessing-%23220/egoio/db_tables. While the release is running changing dependencies might contradict version control in some way. But I guess, it's also important, that this PR after finalization still has to be reviewed, which might take some time too. I guess, it depends on whether there is a next release.

@eosram
Copy link
Author

eosram commented Apr 26, 2018

The pull request is open for review again : ). Forgot to mention that.

@wolfbunke
Copy link
Member

ok, I will check it later

@eosram
Copy link
Author

eosram commented Apr 27, 2018

I just now realized another reason, why storage was commented out up until now and why stay this way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants