Add new tests for seven modules. Include GH workflow. #563

lbonaldo · 2023-10-12T04:37:10Z

Description

This PR adds seven more tests that are executed whenever a pull request is opened, synchronize, and reopened, or at every push targeting main or develop. The tests are:

test_threezones.jl: tests GenX over three zones and five representative days;
test_multistage.jl: tests the multistage module over a single zone and five representative days;
test_electrolyzer.jl: tests the electrolyzer module over five representative days;
test_VREStor.jl: tests VRE and storage over 24 hours;
test_piecewisefuel_CO2.jl: tests the piecewise fuel CO2 module over 24 hours;
test_time_domain_reduction.jl: tests the TDR module;
test_methodofmorris.jl: tests the Method of Morris module.

For the first five cases, the tests are passed if the value of the objective function is consistent over time (up to the tolerance of the solver). The results are stored in a log folder updated in a separate repository (GenXProject/GenX-testlog.git).
The tests in test_time_domain_reduction.jl validate the clustering for the TDR module by computing the rand index and the mutual information and by comparing the output files.
For the Method of Morris module, we are testing if the module is correctly built, and if the output is consistent over time.

I'd appreciate any critiques or suggestions on how to improve these tests.

What type of PR is this?

Feature

Checklist

Code changes are sufficiently documented; i.e. new functions contain docstrings and .md files under /docs/src have been updated if necessary.
The latest changes on the target branch have been incorporated, so that any conflicts are taken care of before merging. This can be accomplished either by merging in the target branch (e.g. 'git merge develop') or by rebasing on top of the target branch (e.g. 'git rebase develop'). Please do not hesitate to reach out to the GenX development team if you need help with this.
Code has been tested to ensure all functionality works as intended.
CHANGELOG.md has been updated (if this is a 'notable' change).
I consent to the release of this PR's code under the GNU General Public license.

How this can be tested

After checking out develop, enter the package manager and run test or test GenX. The tests should run automatically.

Post-approval checklist for GenX core developers

After the PR is approved

Check that the latest changes on the target branch are incorporated, either via merge or rebase
Remember to squash and merge if incorporating into develop

codecov-commenter · 2023-10-12T04:46:10Z

Codecov Report

❗ No coverage uploaded for pull request base (develop@60ca831). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             develop     #563   +/-   ##
==========================================
  Coverage           ?   40.83%           
==========================================
  Files              ?      126           
  Lines              ?     6482           
  Branches           ?        0           
==========================================
  Hits               ?     2647           
  Misses             ?     3835           
  Partials           ?        0

cfe316

This looks great. My comments are mostly cosmetic, except for the possible interactions with Gabe's commerical-solver-use PR (see test/utilities.jl). Approving because I think this should still work.

cfe316 · 2023-10-12T12:43:31Z

.github/workflows/ci.yml

+        version:
+          - "1.6"
+          - "1.7"
+          - "1.8" 


It might be good to ensure we're testing on 1.9 (though I guess this is covered by 1 for "latest"?)

There's some new features in 1.9 that might motivate us to stop supporting older versions in GenX v0.4.

Yes, '1' ensures we're testing on the latest stable release. We could also keep only the current stable release ('1'), and the long-term support (LTS) release (right now, v1.6.7) as other packages do.

cfe316 · 2023-10-12T12:44:58Z

.github/workflows/ci.yml

+            git config --local user.name lbonaldo
+            git config --local user.email "[email protected]"


Kind of random, but I'm a bit surprised to see your name here.

I had thought maybe we had a project-wide github account?

cfe316 · 2023-10-12T12:46:17Z

Project.toml

+JLD2 = "033835bb-8acc-5ee8-8aae-3f567f8a3819"
 Logging = "56ddb016-857b-54e1-b83d-db4d58db5568"
-
+LoggingExtras = "e6f89c97-d47a-5376-807f-9c37f3926c36"
+Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"


10 points for alphabetizing our dependencies! 🎉

cfe316 · 2023-10-12T12:48:48Z

test/Electrolyzer/highs_settings.yml

+# HiGHS Solver Parameters
+# Common solver settings
+Feasib_Tol: 1.0e-04        # Primal feasibility tolerance # [type: double, advanced: false, range: [1e-10, inf], default: 1e-07]
+Optimal_Tol: 1.0e-04       # Dual feasibility tolerance # [type: double, advanced: false, range: [1e-10, inf], default: 1e-07]
+TimeLimit: 1.0e23             # Time limit # [type: double, advanced: false, range: [0, inf], default: inf]
+Pre_Solve: on          # Presolve option: "off", "choose" or "on" # [type: string, advanced: false, default: "choose"]
+Method: ipm             #HiGHS-specific solver settings # Solver option: "simplex", "choose" or "ipm" # [type: string, advanced: false, default: "choose"]
+
+#highs-specific solver settings
+
+# run the crossover routine for ipx
+# [type: string, advanced: "on", range: {"off", "on"}, default: "off"]
+run_crossover: "on"


I'm a bit surprised to see highs_settings.yml in the same folder as the rest of the case. And where is genx_settings.yml? Usually these are both in a Settings subfolder.

Ah okay I think I see how you're doing this below, by using a run_genx_case_testing function. Cool!

cfe316 · 2023-10-12T12:57:00Z

test/test_methodofmorris.jl

+
+# Define test inputs
+genx_setup = Dict(
+    "MacOrWindows" => "Mac",


MaxOrWindows is obsolete and can be removed

cfe316 · 2023-10-12T12:57:53Z

test/test_methodofmorris.jl

+    "ModelingToGenerateAlternatives" => 0,
+    "ModelingtoGenerateAlternativeSlack" => 0.1,
+    "ModelingToGenerateAlternativeIterations" => 3,


Probably these can be removed if they're not being used (since the default is to have them be zero anyway)

cfe316 · 2023-10-12T12:59:42Z

test/test_threezones.jl

+
+include(joinpath(@__DIR__, "utilities.jl"))
+
+obj_true = 6.9602085499e+03


How did you get these numbers? Did you set the solver tolerance to be super super low?
If not, probably we should truncate to the number of digits used by that solver.

I run the example several times and check if the value of the objective function was stable.
You're right, thanks, I will round them to match the tolerance (I'm using Optimal_Tol: 1.0e-05 )

cfe316 · 2023-10-12T13:06:22Z

test/utilities.jl

+function run_genx_case_simple_testing(test_path::AbstractString, genx_setup::Dict)
+    # Run the case
+    OPTIMIZER = configure_solver(genx_setup["Solver"], test_path)
+    inputs = load_inputs(genx_setup, test_path)
+    EP = generate_model(genx_setup, inputs, OPTIMIZER)
+    EP, _ = solve_model(EP, genx_setup)
+    return EP, inputs, OPTIMIZER
+end
+
+function run_genx_case_multistage_testing(test_path::AbstractString, genx_setup::Dict)
+    # Run the case
+    OPTIMIZER = configure_solver(genx_setup["Solver"], test_path)
+
+    model_dict = Dict()
+    inputs_dict = Dict()
+
+    for t in 1:genx_setup["MultiStageSettingsDict"]["NumStages"]
+        # Step 0) Set Model Year
+        genx_setup["MultiStageSettingsDict"]["CurStage"] = t
+
+        # Step 1) Load Inputs
+        inpath_sub = joinpath(test_path, string("Inputs_p", t))
+        inputs_dict[t] = load_inputs(genx_setup, inpath_sub)
+        inputs_dict[t] = configure_multi_stage_inputs(inputs_dict[t], genx_setup["MultiStageSettingsDict"], genx_setup["NetworkExpansion"])


We might consider how this will work with Gabe's optimizer-separating PR.

Spur of the moment idea:
For both, we might consider using keyword arguments. Something like
run_genx_case(path; optimizer:All=HiGHS.Optimizer, genx_setup::Union{nothing,Dict})
and then only what is not already supplied could be loaded.

cfe316 · 2023-10-12T13:06:56Z

test/utilities.jl

+    if !isdir(joinpath("Logs"))
+        mkdir(joinpath("Logs"))
+    end


I think you can just do "Logs" without joinpath

cfe316 · 2023-11-08T16:44:49Z

test/Electrolyzer/Period_map.csv

+Period_Index,Rep_Period,Rep_Period_Index
+1,72,2
+2,72,2
+3,72,2
+4,72,2
+5,72,2
+6,72,2
+7,72,2
+8,72,2
+9,72,2
+10,72,2
+11,72,2
+12,72,2
+13,72,2
+14,72,2
+15,72,2
+16,72,2
+17,72,2
+18,72,2
+19,72,2
+20,72,2
+21,72,2
+22,72,2
+23,72,2
+24,72,2
+25,72,2
+26,72,2
+27,27,1
+28,72,2
+29,72,2
+30,72,2
+31,72,2
+32,72,2


This Period_map file seems odd. Is this 365 days being represented by 5 days?

This PR adds seven more tests to GenX: three-zone, multi-stage, electrolyzer, VRE+storage, piecewise_fuel+CO2, TDR, and Method of Morris. The tests are passed if the value of the objective function is consistent over time (up to the tolerance of the solver). The tests for the TDR validate the clustering by computing the rand index and the mutual information and comparing the output files. For the Method of Morris module, we are testing if the module is correctly built and if the output is consistent over time.

lbonaldo requested review from sambuddhac, cfe316 and gmantegna October 12, 2023 04:37

lbonaldo marked this pull request as draft October 12, 2023 04:37

lbonaldo added this to the v0.4 milestone Oct 12, 2023

lbonaldo added enhancement New feature or request and removed enhancement New feature or request labels Oct 12, 2023

cfe316 approved these changes Oct 12, 2023

View reviewed changes

lbonaldo force-pushed the feature-tests branch from bf40ed4 to deacbab Compare November 8, 2023 15:06

cfe316 reviewed Nov 8, 2023

View reviewed changes

lbonaldo force-pushed the feature-tests branch 2 times, most recently from 593e27a to ff2a8fe Compare November 13, 2023 00:25

lbonaldo added 11 commits November 12, 2023 19:40

Add tests for seven modules. Include GH workflow.

d884829

Add tests for seven modules. Include GH workflow.

b83a3bf

Add rounding to the objective func value in the tests

36a5c01

Removed deprecated setup field

9af3ae0

Change GH action to push to testlog repo

6628a09

Fix permission on ci.yml

c6e03d9

Fix tolerance calculation in tests

3aadd24

Fix 'Can_Retire' column and update Multistage test

a139c29

Update TDR/MoM tests - compare output files

1c99326

Fix compatibility with Julia 1.6

5ced62e

Update CHANGELOG.md

3cc5dea

lbonaldo force-pushed the feature-tests branch from ff2a8fe to 3cc5dea Compare November 13, 2023 01:00

lbonaldo marked this pull request as ready for review November 13, 2023 01:10

Fix setting for tests

a556389

lbonaldo merged commit 8524f4a into GenXProject:develop Nov 13, 2023
7 checks passed

lbonaldo deleted the feature-tests branch June 25, 2024 16:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new tests for seven modules. Include GH workflow. #563

Add new tests for seven modules. Include GH workflow. #563

lbonaldo commented Oct 12, 2023 •

edited

Loading

codecov-commenter commented Oct 12, 2023 •

edited by codecov bot

Loading

cfe316 left a comment

cfe316 Oct 12, 2023

lbonaldo Oct 12, 2023

cfe316 Oct 12, 2023

cfe316 Oct 12, 2023

cfe316 Oct 12, 2023

cfe316 Oct 12, 2023

cfe316 Oct 12, 2023

cfe316 Oct 12, 2023

cfe316 Oct 12, 2023

lbonaldo Oct 12, 2023

cfe316 Oct 12, 2023

cfe316 Oct 12, 2023

cfe316 Nov 8, 2023

lbonaldo Nov 8, 2023

		git config --local user.name lbonaldo
		git config --local user.email "[email protected]"


		include(joinpath(@__DIR__, "utilities.jl"))

		obj_true = 6.9602085499e+03

Add new tests for seven modules. Include GH workflow. #563

Add new tests for seven modules. Include GH workflow. #563

Conversation

lbonaldo commented Oct 12, 2023 • edited Loading

Description

What type of PR is this?

Checklist

How this can be tested

Post-approval checklist for GenX core developers

codecov-commenter commented Oct 12, 2023 • edited by codecov bot Loading

Codecov Report

cfe316 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lbonaldo commented Oct 12, 2023 •

edited

Loading

codecov-commenter commented Oct 12, 2023 •

edited by codecov bot

Loading