OSeMOSYS / otoole

OSeMOSYS Tools for Energy
https://otoole.readthedocs.io
MIT License
23 stars 17 forks source link

[Bug]: `CapitalInvestment` not being calculated #197

Closed trevorb1 closed 7 months ago

trevorb1 commented 11 months ago

The Issue

When converting a result solution file to a folder of CSVs, the variable CapitalInvestment is not written out

Expected Behavior

I would expect the file CapitalInvestment.csv to be available in the folder of results

Steps To Reproduce

  1. clone simplicity
  2. create the datafile with otoole convert csv datafile data data.txt config.yaml
  3. build the model with glpsol -m osemosys_fast.txt -d data.txt --wlp model.lp --check, using the osemosys_fast model
  4. solve the model with cbc model.lp solve -solu model.sol
  5. get results with otoole results cbc csv model.sol results csv data config.yaml
  6. Check the folder of results, and CapitalInvestment.csv is not there :(

Log output

INFO:otoole.results.results:No calculation method available for CapitalInvestment
DEBUG:otoole.results.results:Error calculating CapitalInvestment: "Cannot calculate CapitalInvestment due to missing data: 'DiscountRateIdv is not accessible or available'"

Operating System

Linux

What version of otoole are you running?

1.1.0

Possible Solution

Looks like the result package can not access the data DiscountRateIdv. I think this is set by default to the same as the discount rate, so it may be worth exploring if this behaviour is the same when it is not by default the discount rate.

param DiscountRateIdv{r in REGION, t in TECHNOLOGY}, default DiscountRate[r];

Anything else?

No response

HauHe commented 11 months ago

I have removed the default for the DiscountRateIdv in the model and added it to the data of OSeMBE, where I don't use the DiscountRateIdv. Now otoole produces a csv file for the CapitalInvestment but it is empty. Since I haven't assigned DiscountRateIdv values and set the default to 0.05 as the DiscountRate the product of pv_annuity and CapitalRecoveryFactor should be 1.
Anyhow, I assume the desired behaviour is to also get the CapitalInvestment if no DiscountRateIdv is specified.

willu47 commented 10 months ago

Suggest using DiscountRate if DiscountRateIdv is not available?

HauHe commented 10 months ago

Suggest using DiscountRate if DiscountRateIdv is not available?

That's how I started. Or do you mean in the calculation?

willu47 commented 10 months ago

I mean in the otoole code. The issue is that the results processing code expects DiscountRateIdv to be available. However, we could substitute DiscountRateIdv for DiscountRate in cases where the former is unavailable.

Ultimately, we need to do a bit of think about how to generalise this result processing part of otoole so that it can be better configured to take into account the addition or removal of result parameters, or the use of non-OSeMOSYS models.

HauHe commented 10 months ago

I just wanted to start working on addressing this bug. Kind of accidentally, I ran simplicity with the osemosys code that comes in the simplicity repo, and noticed that when solving the model with CBC otoole produces the two variables CapitalInvestment and TotalDiscountedCost. But when running simplicity with osmeosys_fast and solving with CBC it doesn't. Now I'm wondering, are the two variables calculated when solving when running with the osemosys code in the simplicity repo? And if so why does the calculation work there but not in otoole? Are they different?

HauHe commented 10 months ago

I also noticed, that for me the logger only produces only: INFO:otoole.results.results:Looking for CapitalInvestment INFO:otoole.results.results:No calculation method available for CapitalInvestment in relation to the CapitalInvestment, but not the DEBUG message when running with the -v flag.

HauHe commented 10 months ago

The issue can be addressed by replacing line 311 in result_package.py with:

if "DiscountRateIdv" in self.keys():
                discount_rate_idv = self["DiscountRateIdv"]
else:
                discount_rate_idv = self["DiscountRate"]

However, when using Simplicity then a new issue comes up that I haven't managed to solve yet. In Simplicity the csv for the DiscountRate is empty, because the value is set by the default in the config file. This is I guess not a too rare case since many OSeMOSYS models do not use the set REGION and hence the default value can be used. The problem is, if I'm not mistaken, that otoole doesn't realise that the csv is empty and therefore doesn't use the default value. I haven't found where the DiscountRate is read in, but I guess the issue could be fixed by an if condition checking if the csv is empty and if so use the default.

trevorb1 commented 10 months ago

Hey @HauHe! Apologies for the slow pickup on this. I played around with this briefly and have a couple thoughts.

On your points of:

Now I'm wondering, are the two variables calculated when solving when running with the osemosys code in the simplicity repo?

The Simplicity repo will come with the full version of OSeMOSYS. Therefore, otoole isnt calculating anything in this circumstance.

I also noticed, that for me the logger only produces only in relation to the CapitalInvestment, but not the DEBUG message when running with the -v flag.

If you add -vvv instead of just -v, you should get DEBUG statements?

DEBUG:otoole.results.result_package:Returning 'DiscountRateIdv' from ... 
DEBUG:otoole.results.result_package:  ... Not found in cache or calculation methods
INFO:otoole.results.results:No calculation method available for CapitalInvestment
DEBUG:otoole.results.results:Error calculating CapitalInvestment: "Cannot calculate CapitalInvestment due to missing data: 'DiscountRateIdv is not accessible or available'"

The problem is, if I'm not mistaken, that otoole doesn't realise that the csv is empty and therefore doesn't use the default value.

I agree! It may go even a little further though, in the sense that discount_rate_idv is defined over both REGION and TECHNOLOGY, rather than just REGION like discount_rate. This may cause issues if we just do a simple substitution of discount_rate_idv for discount_rate

I had a couple thoughts on how to solve this, but they didnt really materialize.

  1. Manually creating the discount_rate_idv data in the results package, using the expand_defualts() function. This option is explored here, but didn't really go anywhere since input_data defaults are not accessible in the results package. But maybe a second set of eyes will help. :)
  2. We can manually inject the definition and empty dataframe for discount_rate_idv upon reading in data. However, this would need to be almost at the ReadStrategy class which isn't too elegant of a solution
  3. We can update the config file to describe any parameters that have default definitions defined in the model file

Or in your proposed solution @HauHe, did all the tests pass an whatnot? Im just wondering if any unexpected issues may arise if the indexing for discount_rate_idv is incorrect. But maybe this is an issue for later down the road haha

HauHe commented 10 months ago

Hi @trevorb1, no problem and thanks for picking it up. I'm wondering if there are any other input parameter used in the results processing where OSeMOSYS is relying on the default values like for the DiscountRate and the DiscountRateIdv? Of the top of my had I don't have any current result variables in mind, but I can imagine possible variables that could be added in the future e.g. a variable indicating the load of technologies. In the calculation of such a variable CapacityFactor and AvailabilityFactor would be used, which are also not always both defined for all technologies, hence the default values would be required in the calculation.

I think crucial in a function like your expand_defaults is that the values that are provided are considered and that the default value is only used when no value is provide. E.g. if I define the DiscountRateIdv for some technologies for those technologies those values need to be used, but for the others the default...

I have honestly no idea what would be better where such a function should be placed in otoole. I guess that's rather a discussion for you and @willu47 ?

trevorb1 commented 10 months ago

@HauHe totally agree with your points! I think your example describes a different issue, though? If you define the DiscountRateIdv for some technologies, then you can't use this line in OSeMOSYS, which is the cause of the Capital Investment calculation issue?

param DiscountRateIdv{r in REGION, t in TECHNOLOGY}, default DiscountRate[r];

Maybe it is a bit of a pain, but adding in the definition to the config file and appending this data to the input_values and default_values data during the read in is the easiest option (since this is a bit of an edge case)

Something like:

DiscountRateIdv:
    indices: [REGION,TECHNOLOGY]
    type: param
    default: DiscountRate
HauHe commented 10 months ago

I have tried such a fix, but it only got me an empty CapitalInvestment file. I assume reason being that otoole also in the case can't deal with an empty csv for the DiscountRate

HauHe commented 10 months ago

Anyhow, @trevorb1 have you tested the function you wrote for expanding the default values? Looks like the best option right now, so I would suggest you to implement it.

willu47 commented 10 months ago

@trevorb1 - I think it should be possible to access default values for input config and result config in the ReadResults class. This class inherits from the ReadStrategy. The ReadStrategy is instantiated with a user config. The user config contains default values for inputs and results.

I believe that we could then pass in the default_values for input_data as a dictionary to the ResultsPackage class. This would allow the default values to be available when calculating each of the result parameters if the values for an input are missing. There's a danger that this creates more problems than it fixes, so needs some careful thought. The alternative is to expand all the input parameter files with default values, but this will use a lot of memory unnecessarily as not all input parameters are used for calculating result parameters. I think a lazy approach to filling arrays with default values would be more efficient.

  1. Check if an input file is available
  2. If the file is: a. empty, warn the user, and then fill the array with values from the default_value dict b. full (all index combinations have a value), continue c. partially full (some index combinations are empty), expand the array with the default_value replacing missing data
  3. Perform the calculation
willu47 commented 7 months ago

Just flagging that this issue has also been encountered by colleagues at NTUA.