sedos-project / data_adapter_oemof

This respository holds the data adapters to connect oemof with the OEDatamodel-concrete.
GNU Affero General Public License v3.0
0 stars 1 forks source link

Feature/57 integrate tsam #60

Closed FelixMau closed 5 months ago

FelixMau commented 7 months ago

Integrating TSAM into data_adapter_oemof.


Tsam aggregation can only happen after all adapters have been initialized and all timeseries are available. Functionality will be added to datapackage class within data_adapter_oemof.

Following procedure will be applied:

  1. Save datapackage (maybe incldue a check if it has been saved already?) EDIT This step is skipped. Users shall save it by themself, helping, to be clear about saving locations.
  2. Find all sequences and concat them in one Dataframe
  3. Read Tsam config from Json file
  4. Aggregate seqences.
  5. Save aggregated sequences
  6. Create new datapackage.json file with update resources
FelixMau commented 7 months ago

I still have some open questions regarding naming & other convention where I would like to hear your feedback to:

henhuy commented 7 months ago
  • I also did not quite understand what tsam_config is needed for tabular and moreover what is meant by those variables and where I get them from

Here you can see how you can get needed TSAM config from TSAM aggregation: https://github.com/oemof/oemof-solph/blob/40b95374a45a75b7008e1e559c6c6bcae3d5ad3d/examples/tsam/invest_optimize_all_technologies_using_mp_and_tsam.py#L174-L176

FelixMau commented 7 months ago

Should be faster now, and it really fells faster! Thank you! :) Added tsa parameters to elements which will also add it to the datapackage. I think we do not know yet where exactly the parameters should be, so this was the easiest quick implementation for me. Edit Maybe they are disturbing if found in the datapackage

henhuy commented 7 months ago

Update: I removed need for timeindex in tsa_parameters! Thus, this can be removed from data_adapter and tabular as well, which is nice IMO

nailend commented 7 months ago

Added tsa parameters to elements which will also add it to the datapackage. I think we do not know yet where exactly the parameters should be, so this was the easiest quick implementation for me.

@FelixMau Do you mean in the datapackage? I already implemented it in tabular. So please safe it in a separate directory tsam with filename tsa_parameters.csv

check file-path for location

Thus, this can be removed from data_adapter and tabular as well, which is nice IMO

I think I have never added it if you talke about occurences, as we already talked about this see comment

henhuy commented 7 months ago

I think I have never added it if you talke about occurences, as we already talked about this see comment

No, I'm talking about the timeindex which is not necessary anymore! (As it is now gets disaggregated from period indexes)

nailend commented 7 months ago

No, I'm talking about the timeindex which is not necessary anymore! (As it is now gets disaggregated from period indexes)

Is this a change for the general multi-period?

henhuy commented 7 months ago

No, I'm talking about the timeindex which is not necessary anymore! (As it is now gets disaggregated from period indexes)

Is this a change for the general multi-period?

No, only for our TSAm integration in tabular and adapter

FelixMau commented 7 months ago

I have added a draft of resulting datapackage with tsam integration. Is it as intended?

nailend commented 7 months ago

I have added a draft of resulting datapackage with tsam integration. Is it as intended?

Looks good but can you give the index of data/tsam/tsa_parameters.csv the name period or add an extra column period and save the csv without index please?

nailend commented 7 months ago

And apparently tsam is missing in the pyproject.toml, could you please add it, as well as yourself and Hendrik to the authors? And please update the poetry.lock file afterwards. Thank you :)

FelixMau commented 6 months ago

I think we can merge this tomorrow after a short discussion? I will resolve the conflicts created by my Github misbehavior.

FelixMau commented 6 months ago

Yes the pytest adaption to new data_adapter versions is handled in #59 dtype check should not fail there.

nailend commented 5 months ago

Yes the pytest adaption to new data_adapter versions is handled in #59 dtype check should not fail there.

after merging dtype is still failing. I don't see it as a problem though and will just set it to ignore.