EECA-NZ / TIMES-NZ-Model-Files

Model excel files for TIMES-NZ
MIT License
0 stars 0 forks source link

[python] Run times-excel-reader on TIMES-NZ model #10

Open siddharth-krishna opened 1 year ago

siddharth-krishna commented 1 year ago

This issue tracks progress towards running https://github.com/etsap-TIMES/times-excel-reader/ successfully on TIMES-NZ. (Opening the issue in this repo and not the other one as the latter is public.)

For me the first error is in process_flexible_import_table:

image

https://github.com/etsap-TIMES/times-excel-reader/blob/f6167631a36bb8d30f90ab1ffb235572342e19ab/times_reader/transforms.py#L586-L597

IndexError: index 0 is out of bounds for axis 0 with size 0

The error occurs when process = 'MINWODSUPOSWOD', and this is the offending excel table:

image

That process is not in veda_process_sets:

> [t for t in veda_process_sets['techname'].unique() if t.startswith('MINWOD')]
['MINWODWST00', 'MINWODWST01', 'MINWODSUPCUR00', 'MINWODWST02']

@olejandro , thoughts on how to fix this?

olejandro commented 1 year ago

Thanks @siddharth-krishna for looking into this! Looks like the process in question is not of the expected type. COST should normally be used with IRE processes, but MINWODSUPOSWOD is PRE.

olejandro commented 1 year ago

Opened #kanors-emr/Veda2.0-Installation/issues/16 to see whether to align this with Veda.

olejandro commented 1 year ago

@willcattoneeca got confirmation in the discussion of the issue above, that Veda 2 doesn't have the same behaviour in this respect. For TIMES-NZ model, this means that either:

willcattoneeca commented 1 year ago

Thanks Olex, the timing is pretty good as there is a plan to migrate our configuration files to Veda 2 imminently.

@gg-eeca and @BakytzhanSuleimenov, could you please confirm that Olex's specification for what needs to change makes sense to you?

BakytzhanSuleimenov commented 1 year ago

Thanks @olejandro and @willcattoneeca, yes, sure, this seems like one of the small things that need to be corrected to migrate to Veda 2. I will take care of such things.

BakytzhanSuleimenov commented 1 year ago

Deat @willcattoneeca I have changed the type of the process to "MIN" (mining) from "PRE" (generic process) so with this, we don't need to change "COST" attribute as it works perfectly for such type of process.

willcattoneeca commented 1 year ago

Hi all, I have merged the changes from @BakytzhanSuleimenov, together with the suggestions from @olejandro , to main. Running the times-excel-reader on the files again, I see an error in the same place again, image The table in question this time is image And this time the issue seems to be that the "techname" column (in the dataframe df) contains all None values. An extremely naive attempt to fix this by adding a techname column to the table in the Excel sheet, with values copied from Pset_PN, as per this screenshot, doesn't help: image Does anyone have any advice or thoughts?

olejandro commented 1 year ago

Thanks @willcattoneeca! Actually there should be only one column of that kind, i.e. either techname or pset_pn. The tool will raise an error in the future when both are present. I'll investigate this error.

olejandro commented 1 year ago

The tool should run on the model once #https://github.com/etsap-times/times-excel-reader/issues/123 is merged.

willcattoneeca commented 1 year ago

Can confirm that the tool ran to completion and produced three .dd files when run with --dd argument!

olejandro commented 1 year ago

Great it runs to completion now. Maybe we can use the *.dd files intended for MIRO development to work on the coverage of the tool?

siddharth-krishna commented 11 months ago

Good news, with the changes in https://github.com/etsap-TIMES/times-excel-reader/pull/132 I can run the tool and compare against the DD files:

72.0% of ground truth rows present in output (67385/93604), 9060 additional rows 

Here is the full output of the tool: nz-out.txt