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

Review ind sector #5

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

Review ind sector #5

wants to merge 1 commit into from

Conversation

chrwm
Copy link
Member

@chrwm chrwm commented Jun 6, 2024

Please check the individual commits and commit messages for what is wrong.

General Feedback

  1. The correct version in this state should be: srd_range_draft
  2. Please fix the missing global tables references --> add columns from "missing_col_in_reference_table" to table global_emission_factors

Questions

  1. e.g in ind_automobile_boiler_space_heat_gas_0 your source dict is referencing column 'ef': '7UBA2023a'. Column ef does not exist in the table. Do you mean all columns that start with ef by that?

Generel impressions

You applied the majority of conventions already 😍 Good job!
Please, check for consistency across all your tables additionally to my remarks.

@chrwm chrwm added the review Formal review of data tables label Jun 6, 2024
@anik-ier
Copy link

anik-ier commented Jun 6, 2024

"your source dict is referencing column 'ef': '7UBA2023a'. Column ef does not exist in the table. Do you mean all columns that start with ef by that?"

  • yes, exactly that's what I did with reference! But I guess, this should not be proper way to refer! It should be always with complete column name, right?

@chrwm
Copy link
Member Author

chrwm commented Jun 6, 2024

"your source dict is referencing column 'ef': '7UBA2023a'. Column ef does not exist in the table. Do you mean all columns that start with ef by that?"

* yes, exactly that's what I did with reference! But I guess, this should not be proper way to refer! It should be always with complete column name, right?

Yes, exactly the column_name should be exactly the same, so the complete column name is needed

@anik-ier
Copy link

anik-ier commented Jun 6, 2024

like power sector, I also did not include wacc column for any processes in industry. Should I also include it in every process?

@chrwm
Copy link
Member Author

chrwm commented Jun 6, 2024

like power sector, I also did not include wacc column for any processes in industry. Should I also include it in every process?

The data adapter is currently implemented that wacc needs to be assigned to every process individually.

@anik-ier
Copy link

General Feedback

1. The correct version in this state should be: **srd_range_draft**
2. Please fix the missing global tables references --> add columns from "missing_col_in_reference_table" to table `global_emission_factors `

Questions

1. e.g in [`ind_automobile_boiler_space_heat_gas_0`](https://openenergyplatform.org/dataedit/view/model_draft/ind_automobile_boiler_space_heat_gas_0) your source dict is referencing column `'ef': '7UBA2023a'`. Column `ef` does not exist in the table. Do you mean all columns that start with `ef` by that?

Update:

  1. updated the version from v1 to srd_range_draft
  2. global reference for emission columns are fixed now.
  3. source referencing for every column are provided for Automobile and Chemical industry as I am responsible for these two. As soon as I get source data from Isela, will update also for steel, cement, glass, paper, Aluminum, copper sub-sector.
  4. wacc is assigned for every process.

@anik-ier
Copy link

Hi Christoph @chrwm,

I have uploaded aggregation data for Automobile and Chemical industry. Isela is working on her data aggregation, but it is unlikely she would be able to provide those at the end of this month.
I hope, I am not too late!

Base automatically changed from review-srd-2024-06-04 to main July 22, 2024 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review Formal review of data tables
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants