Model period activity upper limit constraint not satisfied

[vignesh1987]

Hi, The parameter TotalTechnologyModelPeriodActivityUpperLimit is not taken into consideration when the value is set to zero in the following constraint

s.t. TAC2_TotalModelHorizonTechnologyActivityUpperLimit{r in REGION, t in TECHNOLOGY: TotalTechnologyModelPeriodActivityUpperLimit[r,t]>0}: sum{l in TIMESLICE, m in MODEperTECHNOLOGY[t], y in YEAR} RateOfActivity[r,l,t,m,y]*YearSplit[l,y] <= TotalTechnologyModelPeriodActivityUpperLimit[r,t]

If we say that there is no local oil resource in a country and set the TotalTechnologyModelPeriodActivityUpperLimit equal to zero (0) for the technology that produces oil, the constraint is not satisfied. It will be satisfied only if TotalTechnologyModelPeriodActivityUpperLimit >0, which is not the case in this situation. Ideally, the equation should read like the following.

s.t. TAC2_TotalModelHorizonTechnologyActivityUpperLimit{r in REGION, t in TECHNOLOGY}: sum{l in TIMESLICE, m in MODEperTECHNOLOGY[t], y in YEAR} RateOfActivity[r,l,t,m,y]*YearSplit[l,y] <= TotalTechnologyModelPeriodActivityUpperLimit[r,t]

[willu47]

Thanks for the bug report @vignesh1987. What are the implications of removing the TotalTechnologyModelPeriodActivityUpperLimit >0? Would you then need to ensure that there is a value for the RHS for all values or r and t? Could we instead change the > to a >=?

[vignesh1987]

In my opinion, there should not be any implications. If the TotalTechnologyModelPeriodActivityUpperLimit is not specified for some technologies, a default value (high number-999999) is assigned. If we use >=, the problem will still exist as the constraint (s.t. TAC2) will be bypassed for technologies with TotalTechnologyModelPeriodActivityUpperLimit =0.

[willu47]

Excellent, thanks for the explanation

[tniet]

I would agree with Vignesh on this one. In addition to there being no real consequence of changing it, it is also different than the operation of the other similar constraints in the model so should be made consistent.

[willu47]

Great - here's my suggestion for how to use this as a "use case" for best practice.

1. Create a model file which demonstrates the incorrect behaviour when run and identify line in results which is wrong (e.g. a zero value constraint which is exceeded - @vignesh1987 or @tniet or @abhishek0208)
2. Use this to make a test which fails because of the bug in this constraint equation - @willu47
3. Fix the equation so that the test passes - @vignesh1987 or @tniet or @abhishek0208
4. Submit pull request, review and merge in changes - @vignesh1987 or @tniet or @abhishek0208

[tniet]

Sounds like a plan. I'm happy to do this and can create an example that shows the error. I'm not entirely sure how to do this with GitHub - is there a good online tutorial that provides a process I can follow? I assume I have to create a branch (or two) to create the examples, and then show the test results. And then a pull request to get the corrected branch back into the master?

[willu47]

Great @tniet, so the workflow to follow is this:

git checkout master  # make sure you are on the master branch
git pull  # obtain the latest version of the code
git checkout -b <branch name>  # create a new branch off the master branch and then check it out
...
code code code
git add <new files or change files>
git commit -m "A short descriptive commit message"
rinse, repeat as many times as you like/need
...
git push upstream  # Push changes to Github

I'm hoping to automate the tests for stages 2 and 3 above for this, but won't be able to do this for a couple of weeks.

However, getting the model file together and identifying where exactly the incorrect result is would make writing the automated text easy.

[willu47]

Some good learning material for version control with Git

• http://swcarpentry.github.io/git-novice/
• https://coderefinery.github.io/git-collaborative/

[tniet]

Thanks @willu47 - that should get me going. One additional question: We have two repos - the main OSeMOSYS one and the GNU one. I will need to associate the GNU branch I created where I will update code with an updated version of the test case (utopia) in the main repo. How do I do that?

[willu47]

So just work on the GNU MathProg repo, and add the model file for the test case into this repo in a new folder called tests.

[tniet]

Hi @willu47 - I have the changes ready to push back up to GitHub but I get the following error: $ git push upstream fatal: 'upstream' does not appear to be a git repository fatal: Could not read from remote repository.

[tniet]

I pushed it with the following command: git push origin Model_Period_Limit_Test.

[willu47]

@tniet - I have had a look through the commits on your branch. Very nice commit descriptions! I've now setup automated testing using Travis CI which can be viewed in PR #15.

What we need to do now, is to write a test which demonstrates the failing behaviour. For example a test looks like this:

from subprocess import run

class Test_RunMathProg():

    def test_mathprog_run_normal(self):

        arguments = ['glpsol', '-m', 'src/osemosys.txt', '-d', 
                     'tests/utopia.txt', '-o', 'results.csv']
        output = run(arguments, capture_output=True, text=True)
        assert 'OPTIMAL LP SOLUTION FOUND' in output.stdout
        assert 'obj =   2.944686269e+04' in output.stdout

The two assert statements check that the values are found in the text output of the model run.

Alternatively, we could check that particular values exist in the results file (results.csv above).

Perhaps we could write a test together for this bug?

[tniet]

@willu47 - That would be good. I'd like to learn how to automate testing.

I can propose a couple of assert statements that would be true if things were failing. A couple of questions:

1. Where does the above code go?
2. Can the assert statements check number values (non-zero, for example)? Are there some good examples of assert statements that you can point me towards?
3. You mention automated testing for the failing behavior. Do we also need a test for the working behavior?

A heads up that things are pretty busy for me for the rest of August, so I might not have much time to work with this until September. I'll do my best to keep connected to this thread, but my responses might be delayed.

[willu47]

1. Location of the test code:

If you pull down the latest version of the code:

git checkout master
git pull origin master

you should then see the above code in the tests folder. There are instruction in the readme in how to run the tests. Basically, you need a Python environment set up, and to install the pytest package.

Running the tests is as simple as typing the command pytest in the root folder of the OSeMOSYS_GNU_MathProg repository.

2. Assert statements

Assert statements can be used to assert almost anything. We can check number values in the results (although we'll need to write a little utility function to extract the result from the CSV results file), if a file exists, anything really.

The pytest documentation has some examples of assert statements.

One thing to remember is that for a test to pass, the assert statement should return true. So for this bug the assert statement might look something like this assert rate_of_activity == 0.

3. Testing working behaviour

Ideally, all expected behaviour should be backed up with a test. For our purposes, this might include a series of very simple models which demonstrate particular aspects of the model formulation. Each test would run the example model, and check for one or more expected outputs in the results. For example, if 1000 GWh of coal are consumed, check that total CO2 emissions equal 1000 tCO2e.

[tniet]

So, having created a solution to this issue and having dealt with a related issue for a model with another constraint, I have come to realize that the solution I came up with works for this parameter, but that there is a need for a more comprehensive solution to this issue for many different parameters, and that a more generic solution will make OSeMOSYS more efficient.

For this parameter, and others that we don't want to be upper value binding, we currently use a default 9999999 type statement. This means that the solver needs to create a constraint for every possible combination of this equation even for the many cases where the constraint should never bind. The original solution to this was to say that, if the default is 0, don't create the constraint. This leads to the original issue report that you can't set the model period activity to 0.

For many other parameters this is also an issue, and one other factor occasionally exacerbates this issue: If one does not enter enough 9's in the default statement unexpected or unfeasible solutions can occur.

A suggested solution - please chime in with your thoughts - would be to have a default -1 for any upper bound constraint that we want to be non-binding. This allows for 0 level constraints and also allows for non-creation of any equation combinations that should be non-binding. I think this would solve the issue nicely, and also make the model run faster since un-needed constraints are never created.

The main issue with this solution is that it will make older data files incompatible with the new code and that it will require significant re-coding of various parts of the code. Likely something that should be held for a major release and not implemented as a bug fix.

[willu47]

Currently blocked by OSeMOSYS/otoole#1

[abhishek0208]

Can we move this to 'Done' based on #23?