Skip to content

Commit

Permalink
response to review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
jedwards4b committed Aug 31, 2016
1 parent cf2de41 commit 3922258
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 41 deletions.
2 changes: 1 addition & 1 deletion scripts/create_test
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ OR
args.no_batch = True

if args.test_id is None:
args.test_id = CIME.utils.get_utc_timestamp()
args.test_id = CIME.utils.get_timestamp()

if args.testfile is not None:
with open(args.testfile, "r") as fd:
Expand Down
58 changes: 47 additions & 11 deletions utils/python/CIME/hist_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,21 +93,27 @@ def _hists_match(model, hists1, hists2, suffix1="", suffix2=""):
>>> hists1 = ['cam.h0.1850-01-08-00000.nc']
>>> hists2 = ['cam_0001.h0.1850-01-08-00000.nc','cam_0002.h0.1850-01-08-00000.nc']
>>> _hists_match('cam', hists1, hists2, '', '')
([], [], [('cam.h0.1850-01-08-00000.nc', 'cam_0001.h0.1850-01-08-00000.nc')])
([], [], [('cam.h0.1850-01-08-00000.nc', 'cam_0001.h0.1850-01-08-00000.nc'), ('cam.h0.1850-01-08-00000.nc', 'cam_0002.h0.1850-01-08-00000.nc')])
>>> hists1 = ['cam_0001.h0.1850-01-08-00000.nc.base','cam_0002.h0.1850-01-08-00000.nc.base']
>>> hists2 = ['cam_0001.h0.1850-01-08-00000.nc.rest','cam_0002.h0.1850-01-08-00000.nc.rest']
>>> _hists_match('cam', hists1, hists2, 'base', 'rest')
([], [], [('cam_0001.h0.1850-01-08-00000.nc.base', 'cam_0001.h0.1850-01-08-00000.nc.rest'), ('cam_0002.h0.1850-01-08-00000.nc.base', 'cam_0002.h0.1850-01-08-00000.nc.rest')])
"""
normalized1, normalized2 = [], []
multi_normalized1, multi_normalized2 = [], []
multiinst = False
for hists, suffix, normalized in [(hists1, suffix1, normalized1), (hists2, suffix2, normalized2)]:

for hists, suffix, normalized, multi_normalized in [(hists1, suffix1, normalized1, multi_normalized1), (hists2, suffix2, normalized2, multi_normalized2)]:
for hist in hists:
normalized_name = hist[hist.rfind(model):]
if suffix1 != "":
if suffix != "":
expect(normalized_name.endswith(suffix), "How did '%s' not have suffix '%s'" % (hist, suffix))
normalized_name = normalized_name[:len(normalized_name) - len(suffix)]

m = re.search(r"(.*%s.*)_[0-9]{4}(.h.*)"%model, normalized_name)
if m is not None:
multiinst = True
normalized_name = m.group(1)+m.group(2)
multi_normalized.append(m.group(1)+m.group(2))

normalized.append(normalized_name)

Expand All @@ -118,8 +124,31 @@ def _hists_match(model, hists1, hists2, suffix1="", suffix2=""):
two_not_one = sorted([hists2[normalized2.index(item)] for item in set_of_2_not_1])

both = set(normalized1) & set(normalized2)

match_ups = sorted([ (hists1[normalized1.index(item)], hists2[normalized2.index(item)]) for item in both])
if not multiinst:
if multiinst:
new_two_not_one = []
new_one_not_two = []
for normalized_name in two_not_one:
m = re.search(r"(.*%s.*)_[0-9]{4}(.h.*)"%model, normalized_name)
if m is not None:
multi_normalized = m.group(1)+m.group(2)
if multi_normalized in one_not_two:
match_ups.append((multi_normalized, normalized_name) )
else:
new_two_not_one.append(normalized_name)
two_not_one = new_two_not_one
for normalized_name in one_not_two:
m = re.search(r"(.*%s.*)_[0-9]{4}(.h.*)"%model, normalized_name)
if m is not None:
multi_normalized = m.group(1)+m.group(2)
if multi_normalized in two_not_one:
match_ups.append((multi_normalized, normalized_name) )
else:
new_one_not_two.append(normalized_name)
one_not_two = new_one_not_two

else:
expect(len(match_ups) + len(set_of_1_not_2) == len(hists1), "Programming error1")
expect(len(match_ups) + len(set_of_2_not_1) == len(hists2), "Programming error2")

Expand Down Expand Up @@ -155,6 +184,7 @@ def _compare_hists(case, from_dir1, from_dir2, suffix1="", suffix2=""):
all_success = False

num_compared += len(match_ups)

for hist1, hist2 in match_ups:
success, cprnc_comments = cprnc(hist1, hist2, case, from_dir1, multiinst_cpl_compare)
if success:
Expand Down Expand Up @@ -251,7 +281,7 @@ def get_extension(model, filepath):
'0002.h0'
"""
basename = os.path.basename(filepath)
ext_regex = re.compile(r'.*%s[^_]*_?(\d{4})?[.](h.?)([.][^.]+)?[.]nc' % model)
ext_regex = re.compile(r'.*%s[^_]*_?([0-9]{4})?[.](h.?)([.][^.]+)?[.]nc' % model)

m = ext_regex.match(basename)
expect(m is not None, "Failed to get extension for file '%s'" % filepath)
Expand All @@ -268,8 +298,7 @@ def generate_baseline(case, baseline_dir=None, allow_baseline_overwrite=False):
case - The case containing the hist files to be copied into baselines
baseline_dir - Optionally, specify a specific baseline dir, otherwise it will be computed from case config
allow_baseline_overwrite is only used in the CESM model workflow and must be true
to generate baselines to an existing directory.
allow_baseline_overwrite must be true to generate baselines to an existing directory.
returns (SUCCESS, comments)
"""
Expand All @@ -285,7 +314,7 @@ def generate_baseline(case, baseline_dir=None, allow_baseline_overwrite=False):
os.makedirs(basegen_dir)

if (os.path.isdir(os.path.join(basegen_dir,testcase)) and
case.get_value("MODEL") == "CESM" and not allow_baseline_overwrite):
not allow_baseline_overwrite):
expect(False, " Cowardly refusing to overwrite existing baseline directory")

comments = "Generating baselines into '%s'\n" % basegen_dir
Expand Down Expand Up @@ -315,8 +344,15 @@ def generate_baseline(case, baseline_dir=None, allow_baseline_overwrite=False):
comments += " generating baseline '%s' from file %s\n" % (baseline, hist)

# Copy namelist files from CaseDocs
shutil.copytree(os.path.join(case.get_value("CASEROOT"),"CaseDocs"),
os.path.join(basegen_dir,"CaseDocs"))
base_case_docs = os.path.join(basegen_dir,"CaseDocs")
caseroot = case.get_value("CASEROOT")
if os.path.isdir(base_case_docs):
files = os.listdir(os.path.join(caseroot,"CaseDocs"))
for f in files:
shutil.copy2(os.path.join(caseroot, "CaseDocs", f), base_case_docs)
else:
shutil.copytree(os.path.join(case.get_value("CASEROOT"),"CaseDocs"),
os.path.join(basegen_dir,"CaseDocs"))

expect(num_gen > 0, "Could not generate any hist files for case '%s', something is seriously wrong" % testcase)

Expand Down
10 changes: 4 additions & 6 deletions utils/python/CIME/test_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def __init__(self, test_names, test_data=None,
###########################################################################
self._cime_root = CIME.utils.get_cime_root()
self._cime_model = CIME.utils.get_model()

self._allow_baseline_overwrite = allow_baseline_overwrite
self._save_timing = save_timing
self._queue = queue
self._test_data = {} if test_data is None else test_data # Format: {test_name -> {data_name -> data}}
Expand Down Expand Up @@ -80,7 +80,7 @@ def __init__(self, test_names, test_data=None,
self._test_root = self._test_root.replace("$PROJECT", self._project)

self._test_root = os.path.abspath(self._test_root)
self._test_id = test_id if test_id is not None else CIME.utils.get_utc_timestamp()
self._test_id = test_id if test_id is not None else CIME.utils.get_timestamp()

self._compiler = self._machobj.get_default_compiler() if compiler is None else compiler

Expand Down Expand Up @@ -114,11 +114,8 @@ def __init__(self, test_names, test_data=None,
"Missing baseline comparison directory %s" % full_baseline_dir)

# the following is to assure that the existing generate directory is not overwritten
if self._baseline_gen_name and not allow_baseline_overwrite:
if self._baseline_gen_name:
full_baseline_dir = os.path.join(self._baseline_root, self._baseline_gen_name)
expect(not os.path.isdir(full_baseline_dir),
"Refusing to overwrite existing baseline directory, use -o to force overwrite %s" % full_baseline_dir)

else:
self._baseline_root = None

Expand Down Expand Up @@ -368,6 +365,7 @@ def _xml_phase(self, test):
if self._baseline_gen_name:
test_argv += " -generate %s" % self._baseline_gen_name
basegen_case_fullpath = os.path.join(self._baseline_root,self._baseline_gen_name, test)
expect(self._allow_baseline_overwrite or not os.path.isdir(basegen_case_fullpath), "Baseline directory already exists")
logger.warn("basegen_case is %s"%basegen_case_fullpath)
envtest.set_value("BASELINE_NAME_GEN", self._baseline_gen_name)
envtest.set_value("BASEGEN_CASE", os.path.join(self._baseline_gen_name, test))
Expand Down
9 changes: 6 additions & 3 deletions utils/python/CIME/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -489,14 +489,17 @@ def find_proc_id(proc_name=None,

return list(rv)

def get_utc_timestamp(timestamp_format="%Y%m%d_%H%M%S"):
def get_timestamp(timestamp_format="%Y%m%d_%H%M%S", utc_time=False):
"""
Get a string representing the current UTC time in format: YYMMDD_HHMMSS
The format can be changed if needed.
"""
utc_time_tuple = time.gmtime()
return time.strftime(timestamp_format, utc_time_tuple)
if utc_time:
time_tuple = time.gmtime()
else:
time_tuple = time.localtime()
return time.strftime(timestamp_format, time_tuple)

def get_project(machobj=None):
"""
Expand Down
2 changes: 1 addition & 1 deletion utils/python/jenkins_generic_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ def jenkins_generic_job(generate_baselines, submit_to_cdash, no_batch,
batch_args = "--no-batch" if no_batch else ""
pjob_arg = "" if parallel_jobs is None else "-j %d" % parallel_jobs

test_id = "%s_%s" % (test_id_root, CIME.utils.get_utc_timestamp())
test_id = "%s_%s" % (test_id_root, CIME.utils.get_timestamp())
create_test_cmd = "./create_test %s --test-root %s -t %s %s %s %s" % \
(test_suite, test_root, test_id, baseline_args, batch_args, pjob_arg)

Expand Down
39 changes: 20 additions & 19 deletions utils/python/tests/scripts_regression_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ def setUp(self):
self._do_teardown = []

def test_createnewcase(self):
testdir = os.path.join(self._testroot, 'scripts_regression_tests.testcreatenewcase.%s'% CIME.utils.get_utc_timestamp())
testdir = os.path.join(self._testroot, 'scripts_regression_tests.testcreatenewcase.%s'% CIME.utils.get_timestamp())
if os.path.exists(testdir):
shutil.rmtree(testdir)

Expand All @@ -182,7 +182,7 @@ def test_createnewcase(self):
self._do_teardown.append(testdir)

def test_user_mods(self):
testdir = os.path.join(self._testroot, 'scripts_regression_tests.testusermods.%s'% CIME.utils.get_utc_timestamp())
testdir = os.path.join(self._testroot, 'scripts_regression_tests.testusermods.%s'% CIME.utils.get_timestamp())
if os.path.exists(testdir):
shutil.rmtree(testdir)

Expand Down Expand Up @@ -218,10 +218,10 @@ class M_TestWaitForTests(unittest.TestCase):
def setUp(self):
###########################################################################
self._testroot = MACHINE.get_value("CESMSCRATCHROOT")
self._testdir_all_pass = os.path.join(self._testroot, 'scripts_regression_tests.testdir_all_pass.%s'% CIME.utils.get_utc_timestamp())
self._testdir_with_fail = os.path.join(self._testroot, 'scripts_regression_tests.testdir_with_fail.%s'% CIME.utils.get_utc_timestamp())
self._testdir_unfinished = os.path.join(self._testroot, 'scripts_regression_tests.testdir_unfinished.%s'% CIME.utils.get_utc_timestamp())
self._testdir_unfinished2 = os.path.join(self._testroot, 'scripts_regression_tests.testdir_unfinished2.%s'% CIME.utils.get_utc_timestamp())
self._testdir_all_pass = os.path.join(self._testroot, 'scripts_regression_tests.testdir_all_pass.%s'% CIME.utils.get_timestamp())
self._testdir_with_fail = os.path.join(self._testroot, 'scripts_regression_tests.testdir_with_fail.%s'% CIME.utils.get_timestamp())
self._testdir_unfinished = os.path.join(self._testroot, 'scripts_regression_tests.testdir_unfinished.%s'% CIME.utils.get_timestamp())
self._testdir_unfinished2 = os.path.join(self._testroot, 'scripts_regression_tests.testdir_unfinished2.%s'% CIME.utils.get_timestamp())
testdirs = [self._testdir_all_pass, self._testdir_with_fail, self._testdir_unfinished, self._testdir_unfinished2]
for testdir in testdirs:
if os.path.exists(testdir):
Expand Down Expand Up @@ -422,7 +422,7 @@ def setUp(self):
###########################################################################
self._thread_error = None
self._unset_proxy = setup_proxy()
self._baseline_name = "fake_testing_only_%s" % CIME.utils.get_utc_timestamp()
self._baseline_name = "fake_testing_only_%s" % CIME.utils.get_timestamp()
self._machine = MACHINE.get_machine_name()
self._baseline_area = MACHINE.get_value("CCSM_BASELINE")
self._testroot = MACHINE.get_value("CESMSCRATCHROOT")
Expand Down Expand Up @@ -483,10 +483,10 @@ def test_create_test_rebless_namelist(self):
else:
genarg = "-g %s -o"%self._baseline_name
comparg = "-c %s"%self._baseline_name
self.simple_test(True, "%s -n -t %s-%s" % (genarg,self._baseline_name, CIME.utils.get_utc_timestamp()))
self.simple_test(True, "%s -n -t %s-%s" % (genarg,self._baseline_name, CIME.utils.get_timestamp()))

# Basic namelist compare
self.simple_test(True, "%s -n -t %s-%s" % (comparg, self._baseline_name, CIME.utils.get_utc_timestamp()))
self.simple_test(True, "%s -n -t %s-%s" % (comparg, self._baseline_name, CIME.utils.get_timestamp()))

# Modify namelist
fake_nl = """
Expand All @@ -508,13 +508,13 @@ def test_create_test_rebless_namelist(self):
nl_file.write(fake_nl)

# Basic namelist compare should now fail
self.simple_test(False, "%s -n -t %s-%s" % (comparg, self._baseline_name, CIME.utils.get_utc_timestamp()))
self.simple_test(False, "%s -n -t %s-%s" % (comparg, self._baseline_name, CIME.utils.get_timestamp()))

# Regen
self.simple_test(True, "%s -n -t %s-%s" % (genarg, self._baseline_name, CIME.utils.get_utc_timestamp()))
self.simple_test(True, "%s -n -t %s-%s" % (genarg, self._baseline_name, CIME.utils.get_timestamp()))

# Basic namelist compare should now pass again
self.simple_test(True, "%s -n -t %s-%s" % (comparg, self._baseline_name, CIME.utils.get_utc_timestamp()))
self.simple_test(True, "%s -n -t %s-%s" % (comparg, self._baseline_name, CIME.utils.get_timestamp()))

###############################################################################
class O_TestTestScheduler(TestCreateTestCommon):
Expand Down Expand Up @@ -600,7 +600,7 @@ def test_a_phases(self):
def test_b_full(self):
###########################################################################
tests = update_acme_tests.get_full_test_names(["cime_test_only"], self._machine, self._compiler)
test_id="%s-%s" % (self._baseline_name, CIME.utils.get_utc_timestamp())
test_id="%s-%s" % (self._baseline_name, CIME.utils.get_timestamp())
ct = TestScheduler(tests, test_id=test_id, no_batch=NO_BATCH)

build_fail_test = [item for item in tests if "TESTBUILDFAIL." in item][0]
Expand Down Expand Up @@ -728,15 +728,15 @@ def test_jenkins_generic_job(self):
self.simple_test(True, "-t cime_test_only_pass -g -b %s" % self._baseline_name)
self.assert_num_leftovers()

build_name = "jenkins_generic_job_pass_%s" % CIME.utils.get_utc_timestamp()
build_name = "jenkins_generic_job_pass_%s" % CIME.utils.get_timestamp()
self.simple_test(True, "-t cime_test_only_pass -b %s" % self._baseline_name, build_name=build_name)
self.assert_num_leftovers() # jenkins_generic_job should have automatically cleaned up leftovers from prior run
assert_dashboard_has_build(self, build_name)

###########################################################################
def test_jenkins_generic_job_kill(self):
###########################################################################
build_name = "jenkins_generic_job_kill_%s" % CIME.utils.get_utc_timestamp()
build_name = "jenkins_generic_job_kill_%s" % CIME.utils.get_timestamp()
run_thread = threading.Thread(target=self.threaded_test, args=(False, " -t cime_test_only_slow_pass -b master --baseline-compare=no", build_name))
run_thread.daemon = True
run_thread.start()
Expand Down Expand Up @@ -790,16 +790,16 @@ def test_bless_test_results(self):
genarg = "-g %s -o"%self._baseline_name
comparg = "-c %s"%self._baseline_name

self.simple_test(True, "%s -t %s-%s" % (genarg, self._baseline_name, CIME.utils.get_utc_timestamp()))
self.simple_test(True, "%s -t %s-%s" % (genarg, self._baseline_name, CIME.utils.get_timestamp()))

# Hist compare should pass
self.simple_test(True, "%s -t %s-%s" % (comparg, self._baseline_name, CIME.utils.get_utc_timestamp()))
self.simple_test(True, "%s -t %s-%s" % (comparg, self._baseline_name, CIME.utils.get_timestamp()))

# Change behavior
os.environ["TESTRUNDIFF_ALTERNATE"] = "True"

# Hist compare should now fail
test_id = "%s-%s" % (self._baseline_name, CIME.utils.get_utc_timestamp())
test_id = "%s-%s" % (self._baseline_name, CIME.utils.get_timestamp())
self.simple_test(False, "%s -t %s" % (comparg, test_id))

# compare_test_results should detect the fail
Expand All @@ -816,7 +816,7 @@ def test_bless_test_results(self):
run_cmd_no_fail("%s/bless_test_results --hist-only --force -b %s -t %s" % (TOOLS_DIR, self._baseline_name, test_id))

# Hist compare should now pass again
self.simple_test(True, "%s -t %s-%s" % (comparg, self._baseline_name, CIME.utils.get_utc_timestamp()))
self.simple_test(True, "%s -t %s-%s" % (comparg, self._baseline_name, CIME.utils.get_timestamp()))

###############################################################################
@unittest.skip("Disabling this test until we figure out how to integrate ACME tests and CIME xml files.")
Expand Down Expand Up @@ -1007,6 +1007,7 @@ def setUp(self):
self._do_teardown = []

testdir = os.path.join(self._testroot, 'scripts_regression_tests.testscripts.%s'% CIME.utils.get_utc_timestamp())

if os.path.exists(testdir):
shutil.rmtree(testdir)

Expand Down

0 comments on commit 3922258

Please sign in to comment.