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.

Sign up for GitHub

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

Jump to bottom

Add new 'perf' test property #4486

Merged
merged 3 commits into from
Sep 11, 2023
Merged

Add new 'perf' test property #4486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c92091d
Add new 'perf' test property
jgfouca Sep 8, 2023
32f3ea2
Minor fixes so that the cime_test_perf suite is actually runnable
jgfouca Sep 8, 2023
45a0794
pylint fix
jgfouca Sep 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 36 additions & 3 deletions CIME/get_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
# "inherit" : (suite1, suite2, ...), # Optional. Suites to inherit tests from. Default is None. Tuple, list, or str.
# "time" : "HH:MM:SS", # Optional. Recommended upper-limit on test time.
# "share" : True|False, # Optional. If True, all tests in this suite share a build. Default is False.
# "perf" : True|False, # Optional. If True, all tests in this suite will do performance tracking. Default is False.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait every test in a suite? We have a command line argument to create_test for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rljacob , yes:

The idea would be to select a few tests in the integration suite, pull them into their own suite, mark them as perf=True, then have the original suite inherit the new suite.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you would still need 2 separate blocks in tests.py but they could all be launched with one create_test command?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rljacob , yes. It has to be this way (for this perf=True approach) since we store properties on test suites, not individual tests. Another option would be to add a testmod that turns on performance tracking.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take a look at how e3sm_integration is currently assembled. Its a combo of suites which are themselves combos of suites and each of those may have to be broken up in to perf and no-perf suites.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's expected. I think only a few tests will become performance tests.

# "tests" : (test1, test2, ...) # Optional. The list of tests for this suite. See above for format. Tuple, list, or str. This is the ONLY inheritable attribute.
# }

Expand Down Expand Up @@ -89,6 +90,16 @@
"SMS_P16.f19_g16_rx1.X",
),
},
"cime_test_perf": {
"time": "0:10:00",
"perf": True,
"tests": (
"SMS_P2.T42_T42.S",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean here - just change SMS to PFS.

"SMS_P4.T42_T42.S",
"SMS_P8.T42_T42.S",
"SMS_P16.T42_T42.S",
),
},
"cime_test_repeat": {
"tests": (
"TESTRUNPASS_P1.f19_g16_rx1.A",
Expand Down Expand Up @@ -159,19 +170,20 @@ def _get_key_data(raw_dict, key, the_type):
def get_test_data(suite):
###############################################################################
"""
For a given suite, returns (inherit, time, share, tests)
For a given suite, returns (inherit, time, share, perf, tests)
"""
raw_dict = _ALL_TESTS[suite]
for key in raw_dict.keys():
expect(
key in ["inherit", "time", "share", "tests"],
key in ["inherit", "time", "share", "perf", "tests"],
"Unexpected test key '{}'".format(key),
)

return (
_get_key_data(raw_dict, "inherit", tuple),
_get_key_data(raw_dict, "time", str),
_get_key_data(raw_dict, "share", bool),
_get_key_data(raw_dict, "perf", bool),
_get_key_data(raw_dict, "tests", tuple),
)

Expand Down Expand Up @@ -201,7 +213,7 @@ def get_test_suite(
"Compiler {} not valid for machine {}".format(compiler, machine),
)

inherits_from, _, _, tests_raw = get_test_data(suite)
inherits_from, _, _, _, tests_raw = get_test_data(suite)
tests = []
for item in tests_raw:
expect(
Expand Down Expand Up @@ -298,6 +310,27 @@ def get_build_groups(tests):
return [tuple(item[0]) for item in build_groups]


###############################################################################
def is_perf_test(test):
###############################################################################
"""
Is the provided test in a suite with perf=True?

>>> is_perf_test("SMS_P2.T42_T42.S.melvin_gnu")
True
>>> is_perf_test("SMS_P2.f19_g16_rx1.X.melvin_gnu")
False
"""
# Get a list of performance suites
suites = get_test_suites()
for suite in suites:
perf = get_test_data(suite)[3]
if perf and suite_has_test(suite, test, skip_inherit=True):
return True

return False


###############################################################################
def infer_arch_from_tests(testargs):
###############################################################################
Expand Down
7 changes: 5 additions & 2 deletions CIME/test_scheduler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from collections import OrderedDict

from CIME.XML.standard_module_setup import *
from CIME.get_tests import get_recommended_test_time, get_build_groups
from CIME.get_tests import get_recommended_test_time, get_build_groups, is_perf_test
from CIME.utils import (
append_status,
append_testlog,
Expand Down Expand Up @@ -967,7 +967,10 @@ def _xml_phase(self, test):
)
envtest.set_initial_values(case)
case.set_value("TEST", True)
case.set_value("SAVE_TIMING", self._save_timing)
if is_perf_test(test):
case.set_value("SAVE_TIMING", True)
else:
case.set_value("SAVE_TIMING", self._save_timing)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the key change I think. E3SM outputs a bunch of extra perf data and, if you use create_test, none of it is saved. If you use create_newcase, its saved to a special directory on each machine. You can override that with a command-line argument but that would do it for every test in a suite. This allows another way to save the timing info from single tests in a large suite (our integration suite has 100+ cases). Instead of creating/running a separate suite just for performance.


# handle single-exe here, all cases will use the EXEROOT from
# the first case in the build group
Expand Down
Loading