Py regex kdr #2338

jedwards4b · 2018-03-02T21:44:33Z

Proposed changes to st_archive regular expressions,
which accommodate DART files, reduce metacharacters
in env_archive.xml, and make some regexes more selective
or robust.
Potential fix for #2334.
I'm not sure that e3sm changes should be in this,
but included them to be sure the regression test would work.

Test suite: scripts_regression_tests.py
Test baseline:
Test namelist changes:
Test status: bit for bit

Fixes #2334

User interface changes?: in env_archive.xml

Update gh-pages html (Y/N)?:

Code review: gold2718, jedwards4b, mfdeakin-sandia

… DART files, reduce metacharacters in env_archive.xml, and make some regexes more selective or robust. Potential fix for #2334. I'm not sure that e3sm changes should be in this, but included them to be sure the regression test would work.

…ve.xml.

jedwards4b

It looks to me like you took a portion of the regular expressions out of xml and added it to the code. Why?

jedwards4b · 2018-03-02T21:47:08Z

scripts/lib/CIME/case_st_archive.py

@@ -313,19 +314,20 @@ def _archive_history_files(case, archive, archive_entry,
            if ninst_string:
                if compname.find('mpas') == 0:
                    # Not correct, but MPAS' multi-instance name format is unknown.
-                    newsuffix = compname + '.*' + suffix
+                    newsuffix =                   compname + '\d*'                   + '\.' + suffix + '\.'


why all of the additional whitespace here?

jedwards4b · 2018-03-02T22:59:51Z

This is actually @kdraeder work but I opened the PR for him.

jgfouca · 2018-03-02T23:00:53Z

Passing the buck to @mfdeakin-sandia

gold2718

I'm not sure why I am on the reviewer list. I do not know enough about the short term archiver to be dangerous, just clumsy.

gold2718 · 2018-03-03T02:56:19Z

config/cesm/config_archive.xml

+    <rest_file_extension>[ri]</rest_file_extension>
+    <rest_file_extension>rh\d*</rest_file_extension>
+    <rest_file_extension>rs</rest_file_extension>
+    <hist_file_extension>[eh]</hist_file_extension>


What is the 'e' for?

Do you have an example where ESP files are showing up where CAM or CLM files belong?

gold2718 · 2018-03-03T02:59:09Z

scripts/lib/CIME/case_st_archive.py

+#     That could be done with r"\"\.*\S+\s?\""
+#     which is                 '"...{1 or more non-space}{optional space}"'
+# An example that matches the simpler pattern:
+#    "/glade/scratch/raeder/CIME_DA_vars_6/run/CIME_DA_vars_6.cam_0001.rs.2008-08-02-21600.nc"


This needs to be resolved before merging.

gold2718 · 2018-03-03T02:59:28Z

scripts/lib/CIME/case_st_archive.py

+#                 pattern = r"{}\.{}\d*.*".format(casename, compname)
+# KDR This finds   casename.compname[any # digits][any # non-Ret chars]
+#     Instead want casename.compname[any # digits or _].[any # non-Ret chars]
+                pattern = r"{}.{}[\d_]*\..*".format(casename, compname)


This needs to be resolved before merging.

kdraeder · 2018-03-05T03:28:59Z

@goldy was nominated as a reviewer because the(?) resident st_archive expert (Alice) is unavailable, @jedwards4 is the assignee, @kdraeder is the author, and we don't know who else is interested in reviewing DART related PRs. 'e' is the file type for ESP files, some of which end up in CAM, CLM, ... archive directories, but are not any other file type like i, r, h#, ... I think that the 3rd item Steve flagged doesn't require much discussion. The multi-instance file names have component parts like cam_0001, so '_' needs to be included in the allowable characters that follow $compname before the next '.' . Kevin

…

On Fri, Mar 2, 2018 at 8:00 PM, goldy ***@***.***> wrote: ***@***.**** requested changes on this pull request. I'm not sure why I am on the reviewer list. I do not know enough about the short term archiver to be dangerous, just clumsy. ------------------------------ In config/cesm/config_archive.xml <#2338 (comment)>: > @@ -1,10 +1,10 @@ <components version="2.0"> <comp_archive_spec compname="cam" compclass="atm"> - <rest_file_extension>\.[ri]\..*</rest_file_extension> - <rest_file_extension>\.rh\d\.*</rest_file_extension> - <rest_file_extension>\.rs\.*</rest_file_extension> - <hist_file_extension>\.h.*.nc$</hist_file_extension> + <rest_file_extension>[ri]</rest_file_extension> + <rest_file_extension>rh\d*</rest_file_extension> + <rest_file_extension>rs</rest_file_extension> + <hist_file_extension>[eh]</hist_file_extension> What is the 'e' for? ------------------------------ In scripts/lib/CIME/case_st_archive.py <#2338 (comment)>: > @@ -366,6 +368,12 @@ def get_histfiles_for_restarts(rundir, archive, archive_entry, restfile): for item in items: # the following match has an option of having a './' at the beginning of # the history filename +# KDR Does this do what's intended? +# I think it's looking for '"...//{awordchar}{anythingelse} "' +# That could be done with r"\"\.*\S+\s?\"" +# which is '"...{1 or more non-space}{optional space}"' +# An example that matches the simpler pattern: +# "/glade/scratch/raeder/CIME_DA_vars_6/run/CIME_DA_vars_6.cam_0001.rs.2008-08-02-21600.nc" This needs to be resolved before merging. ------------------------------ In scripts/lib/CIME/case_st_archive.py <#2338 (comment)>: > @@ -465,16 +473,19 @@ def _archive_restarts_date_comp(case, archive, archive_entry, pfile = re.compile(pattern) restfiles = [f for f in os.listdir(rundir) if pfile.search(f)] else: - pattern = r"{}\.{}\d*.*".format(casename, compname) +# pattern = r"{}\.{}\d*.*".format(casename, compname) +# KDR This finds casename.compname[any # digits][any # non-Ret chars] +# Instead want casename.compname[any # digits or _].[any # non-Ret chars] + pattern = r"{}.{}[\d_]*\..*".format(casename, compname) This needs to be resolved before merging. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2338 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMpTEAlQFQnlHzjvYkUi_f82IBxyWd0aks5tagdogaJpZM4SairU> .

gold2718 · 2018-03-05T04:14:27Z

@kdraeder, it would help if you would enter your comments on the github page where my comments are so I can be sure we are talking about the same thing. Please look there for my updates.

kdraeder · 2018-03-05T04:36:27Z

@goldy,
I thought that the discussion would happen under the issue,
so I reloaded #2334
and did not see your comments, so I figured they were a direct
response to @jedwards4 original message. I apologize for
being high maintenance again. Now I've found the way to see
the issue comments.

…ure issue about DART restart files.

kdraeder · 2018-03-05T15:42:45Z

@gold2718
I removed the comments that prompted your 2nd comment,
about the restart history file regex, so that's resolved for now.
My push put the comments into my response about not answering
comments in the right way, which seems odd. Is there a way that
I can make my push comments appear as new, instead attaching
to whatever I or someone last wrote?

gold2718 · 2018-03-05T16:01:36Z

@kdraeder, I see the notification about your push right below the last comment which is where it is supposed to be (so to me, it looks like new top-level item).
Comments that are specifically about the PR go in the PR, not in the issue. I would phrase it as the 'Issue discussion' is to clarify the issue and discuss proposed solutions while the 'PR discussion' is for comments on the requested merge. Now I have to figure out a good place to put that on the wiki.

gold2718

It looks like case.st_archive.py still has a KDR comment.

gold2718 · 2018-03-05T16:04:36Z

config/cesm/config_archive.xml

+    <hist_file_extension>h\d*</hist_file_extension>
+    <hist_file_extension>initial_hist</hist_file_extension>
+      <rest_history_varname>unset</rest_history_varname>
+      <rpointer>


Are these spaces correct? I think that <rpointer_file> and <rpointer_content> should be nested under <rpointer> but <rpointer> and <rest_history_varname> are at the same level as <hist_file_extension>

mfdeakin-sandia

Aside from @gold2718 's request for the comment to be resolved and the other spacing issues, this looks fine to me

mfdeakin-sandia · 2018-03-05T22:04:03Z

scripts/lib/CIME/case_st_archive.py

@@ -313,19 +314,20 @@ def _archive_history_files(case, archive, archive_entry,
            if ninst_string:
                if compname.find('mpas') == 0:
                    # Not correct, but MPAS' multi-instance name format is unknown.
-                    newsuffix = compname + '.*' + suffix
+                    newsuffix =                   compname + r'\d*'                   + r'\.' + suffix + r'\.'


Some more spacing issues

I did that to vertically line up the same elements in multiple lines,
to make it easier to see the differences between lines. If this
goes against the CIME style standards, I'll remove the extra spaces.

For the record, I find no issue with these spaces (they do increase readability IMHO).

It's not my preference (personally I prefer a formatting tool to do my formatting thinking for me), but if others don't have a problem with it, then that's fine. Though the one on line 322 is missing a space.

@mfdeakin-sandia, does your formatting tool change these spaces? If not, what is the issue?
I think our python standard is 4 spaces for each indent level and no trailing whitespace.

No issue, I'm fine with this PR

gold2718

I still would like to understand the need for the [eh] hist_file_extension for CAM.

kdraeder · 2018-03-06T20:42:11Z

[eh] refers to 2 different file types. 'e' denotes files that were generated by esp (DART)
and have characteristics of a history file, but are not CAM history files. They are closely
associated with CAM ensemble members (instances).
They are archived the same way as history files, in $DOUT_S_ROOT/atm/hist.
Example filenames are
CIME_DA_vars_11.cam_0001.e.postassim.2008-08-02-64800.nc
CIME_DA_vars_11.cam_0001.e.preassim.2008-08-02-43200.nc
where 'preassim' and 'postassim' refer to output from different stages
of the assimilation process.

gold2718 · 2018-03-06T20:56:02Z

So who outputs CIME_DA_vars_11.cam_0001.e.postassim.2008-08-02-64800.nc?

kdraeder · 2018-03-06T20:59:43Z

The program 'filter', which is run by assimilate.csh.

gold2718 · 2018-03-06T21:02:40Z

Then why is the entry under <comp_archive_spec compname="cam" compclass="atm"> instead of <comp_archive_spec compclass="esp" compname="dart">?

kdraeder · 2018-03-06T21:15:32Z

Because, as described in earlier discussions about the strategy
for archiving CAM+DART files, I chose to keep DART files that
describe an ensemble member with the CAM files that describe the
same member. Users will want to compare these files, and it will be
helpful to have them in the same directory. Also, the CESM file naming
convention and file name construction in case_st_archive.py put restrictions
on what file names can be archived by which sections of env_archive.xml.
A file name with cam_0001 will not be archived by
<comp_archive_spec compclass="esp" compname="dart">.
The file name would need to have dart_0001 in it, which would make
things more cumbersome when we do coupled assimilations which
will generate cam_0001, pop_0001, clm_0001, ... files. If they all have
dart_0001 in their filenames, then the component part will need to be
wedged in somewhere else, making the names even longer, and no more
informative.

There are other DART files that describe the whole ensemble; those are
archived in esp/hist.

gold2718 · 2018-03-07T04:53:00Z

Ah, thanks.

kdraeder added 2 commits March 2, 2018 10:49

Fix accidental changes imported from the cesm version of config_archi…

7b3fb74

…ve.xml.

jedwards4b added the in progress label Mar 2, 2018

jedwards4b mentioned this pull request Mar 2, 2018

Py regex kdr CESM-Development/cime#499

Closed

jedwards4b commented Mar 2, 2018

View reviewed changes

fix pylint issue

496c2ce

jedwards4b self-assigned this Mar 2, 2018

jedwards4b requested review from gold2718 and jgfouca March 2, 2018 22:55

jgfouca requested review from mfdeakin-sandia and removed request for jgfouca March 2, 2018 23:00

gold2718 requested changes Mar 3, 2018

View reviewed changes

Remove question about restart history file regex. It belongs in a fut…

fcc1126

…ure issue about DART restart files.

gold2718 requested changes Mar 5, 2018

View reviewed changes

Fixed space error.

5d37892

mfdeakin-sandia requested changes Mar 5, 2018

View reviewed changes

kdraeder added 2 commits March 6, 2018 09:47

Added a couple missing spaces for readability.

3c2f819

Deleted unneeded comment explaining a regex change.

deb4dd7

mfdeakin-sandia approved these changes Mar 6, 2018

View reviewed changes

gold2718 requested changes Mar 6, 2018

View reviewed changes

gold2718 approved these changes Mar 7, 2018

View reviewed changes

gold2718 assigned gold2718 and unassigned jedwards4b Mar 7, 2018

gold2718 merged commit df4f779 into ESMCI:master Mar 7, 2018

rljacob removed the in progress label Mar 7, 2018

kdraeder deleted the py_regex_kdr branch March 7, 2018 05:45

rljacob mentioned this pull request Mar 22, 2018

Mosart rh0 files no longer archived #2394

Closed

kdraeder mentioned this pull request Apr 9, 2018

config_archive.xml syntax error #2194

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Py regex kdr #2338

Py regex kdr #2338

jedwards4b commented Mar 2, 2018 •

edited by gold2718

Loading

jedwards4b left a comment

jedwards4b Mar 2, 2018

jedwards4b commented Mar 2, 2018

jgfouca commented Mar 2, 2018

gold2718 left a comment

gold2718 Mar 3, 2018

gold2718 Mar 5, 2018

gold2718 Mar 3, 2018

gold2718 Mar 3, 2018

kdraeder commented Mar 5, 2018 via email •

edited

Loading

gold2718 commented Mar 5, 2018

kdraeder commented Mar 5, 2018

kdraeder commented Mar 5, 2018

gold2718 commented Mar 5, 2018

gold2718 left a comment

gold2718 Mar 5, 2018

mfdeakin-sandia left a comment

mfdeakin-sandia Mar 5, 2018

kdraeder Mar 5, 2018

gold2718 Mar 5, 2018

mfdeakin-sandia Mar 6, 2018

gold2718 Mar 6, 2018

mfdeakin-sandia Mar 6, 2018

gold2718 left a comment

kdraeder commented Mar 6, 2018

gold2718 commented Mar 6, 2018

kdraeder commented Mar 6, 2018

gold2718 commented Mar 6, 2018

kdraeder commented Mar 6, 2018

gold2718 commented Mar 7, 2018

Py regex kdr #2338

Py regex kdr #2338

Conversation

jedwards4b commented Mar 2, 2018 • edited by gold2718 Loading

jedwards4b left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jedwards4b commented Mar 2, 2018

jgfouca commented Mar 2, 2018

gold2718 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

kdraeder commented Mar 5, 2018 via email • edited Loading

gold2718 commented Mar 5, 2018

kdraeder commented Mar 5, 2018

kdraeder commented Mar 5, 2018

gold2718 commented Mar 5, 2018

gold2718 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mfdeakin-sandia 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

gold2718 left a comment

Choose a reason for hiding this comment

kdraeder commented Mar 6, 2018

gold2718 commented Mar 6, 2018

kdraeder commented Mar 6, 2018

gold2718 commented Mar 6, 2018

kdraeder commented Mar 6, 2018

gold2718 commented Mar 7, 2018

jedwards4b commented Mar 2, 2018 •

edited by gold2718

Loading

kdraeder commented Mar 5, 2018 via email •

edited

Loading