Implements changes requested by @cameronsmith1 and @PeterCaldwell

[mfdeakin-sandia]

Restored functionality to delete of run and build directories, and reuse other builds while still allowing CIME defaults to be used.
Merged in PMC's changes from 3.0.4. Enabled using CIME defaults for more functionality.
Renamed 'print' and 'newline' to 'acme_print' and 'acme_newline' to disambiguate them from system commands.
BFB, passes tests by hand

[mfdeakin-sandia]

Let me know if there are any other changes you'd like

[cameronsmith1]

@mfdeakin-sandia , I found a weakness with your print alias. We often want to print helpful information on shell variables, eg:

acme_print '$fetch_code = '$fetch_code

However, the initial string gets evaluate, so the output is something like:

false = false

I believe this can be fixed by changing the alias to:

alias acme_print 'echo run_acme: \!*' (the backslash in this line originally didn't show. Fixed 2017-4-10)

[cameronsmith1]

I would like to change seconds_before_delete_case_dir to

seconds_before_delete_case_dir = 10

If it is set to 1 second, a new user won't figure out what it is happening in time to cancel it.

[cameronsmith1]

Unfortunately, github is not responding when I try to scroll through the changes, so I cannot check everything. Hopefully this will get resolved soon.

[cameronsmith1]

I changed browser, and now github is working again for me.

+) Can you shift script_ver above the alias section (so it is easier to find). This means changing acme_print to echo, but it is obvious that it refers to run_acme.
+) Line 549 (pwd) looks like it is a temporary debugging line.
+) Lines 869-872 in previous script: Why has the setting of DOUT_L_MSROOT been removed?

[mfdeakin-sandia]

I've implemented but haven't tested the changes you've requested; let me know if that's everything

[PeterCaldwell]

Looks good! I particularly like how you've set the default code_root_dir. I didn't have time to actually run the code, but I did look through your changes. Two comments which don't affect functionality but will clean/clarify things:

1. There's a special "if anvil" loop around line 891 which can now be deleted since umask is now set to give everyone read permissions.
2. line 847 (append " -cosp"): I'd like a comment above this line noting that xmlchange --append has a bug in it where you need to specify the thing you're appending in quotes with the first character being a space.

[mfdeakin-sandia]

@PeterCaldwell Done. I also removed setting the BFBFLAG, as this is targetting master so it's redundant.

[PeterCaldwell]

Awesome, thanks for the changes. Thanks also for removing the unneeded BFB_FLAG fix. I think this new version is ready for master.

[mfdeakin-sandia]

Unfortunately not quite yet - acme_print is giving me weird output that I don't understand. I think '!*' is the issue; as my output looks like "run_acme: wojr frjlk ${thing_to_print}".
Would there be another way to deal with printing parameters?

[mfdeakin-sandia]

I've managed to fix it by reverting to the alias I had and changing variable names we wanted to print to \$varname outside of quotes. Hacky, but at least it no longer prints the last thing in your csh history file (in my case from testing the alias)

[cameronsmith1]

Hi @mfdeakin-sandia , there should have been a backslash in my acme_print alias, however after pasting it into the github comment I didn't notice that the slash disappeared when github formatted it. The correct line should be:

alias acme_print 'echo run_acme: \!*'

[mfdeakin-sandia]

Ah, that's much better :)
I'll undo the previous commit and add the backslash

[cameronsmith1]

(ISSUE A) I am testing the latest run_acme script with the default options. I have encountered a few minor problems. One issue is that ACME cannot find some data files, and cannot download them (on Edison):

/project/projectdirs/acme/inputdata/ice/mpas-cice/oQU240/mpas-cice.graph.info.151209.part.960
/project/projectdirs/acme/inputdata/ocn/mpas-o/oQU240/mpas-o.graph.info.151209.part.960

UPDATE: These files don't exist on the SVN server. However, the root problem is that the default PE layout uses 960 PEs, which seems unreasonable for the QU240 grid ( approx. 2.4 deg ), so the graph.info files weren't created.

Does anyone have a PE file for Edison for this resolution?

[cameronsmith1]

(ISSUE B) I just encountered the following error when setting old_executable=true

WARNING: run_acme must be run interactively when using the default build directory and old executables
To remedy this, either set $case_build_dir to the path of the executables
If this is an interactive shell, deactivate this test

I do not understand what problem this is trying to catch, nor the solution. Can you clarify?

[cameronsmith1]

(ISSUE C) The default location for the case_scripts directory is now in the directory containing the run_acme script (It used to be in the directory with the bld and run directories). Is this change intentional? If so, what is the purpose?

[PeterCaldwell]

Issue B response: old_executable worked for me last week in SimulationScripts version 1.04(?). I don't recall adding this error message. @mfdeakin-sandia - did you add this check?

[cameronsmith1]

(ISSUE D) I just pushed a small functional change, along with fixing some typos (including one that stopped the code from completing).

I have other small changes I would like to make, but wanted to push what I have so far.

Comments are welcome.

[cameronsmith1]

(ISSUE E) Why is debug_queue=false by default? I would have expected we would want it to be true by default.

I also don't understand the usage of '=~' for the related error message (see below). I think =~ is basically the same as ==, so that what we would want is !~ or !=, but what we really want is walltime > 30 , but I don't know an easy way to do it. (suggestions?)

if ( lowercase $debug_queue == true && walltime =~ 30:00 ) then
acme_print ' You are running in the debug queue and asked for walltime = '$walltime
acme_print ' Generally, only walltime of 30 min or less is allowed in debug queue.'
acme_print ' If you intentionally asked for walltime different than 30 min, disable'
acme_print ' this error message. Quitting.'
exit 18
endif

[PeterCaldwell]

EDIT2: The version below should work now - I had to add MAX_TASKS_PER_NODE and PES_PER_NODE to accommodate changes in CIME behavior.

Issue A: yes, I've struggled unsuccessfully to get reasonable default F compsets on Edison. I've got a 114 node layout I've been using that gets ~9 SYPD. Add a "custom114" option to the custom PE layout in the run script:

else if ( lowercase $processor_config == 'custom114' ) then

./xmlchange -file env_mach_pes.xml MAX_TASKS_PER_NODE="48"
./xmlchange -file env_mach_pes.xml PES_PER_NODE="24"

$xmlchange_exe --id NTASKS_ATM --val 1350
$xmlchange_exe --id NTHRDS_ATM --val 4
$xmlchange_exe --id ROOTPE_ATM --val 0

$xmlchange_exe --id NTASKS_LND --val 984
$xmlchange_exe --id NTHRDS_LND --val 4
$xmlchange_exe --id ROOTPE_LND --val 384

$xmlchange_exe --id NTASKS_ICE --val 384
$xmlchange_exe --id NTHRDS_ICE --val 1
$xmlchange_exe --id ROOTPE_ICE --val 0

$xmlchange_exe --id NTASKS_OCN --val 256
$xmlchange_exe --id NTHRDS_OCN --val 1
$xmlchange_exe --id ROOTPE_OCN --val 0

$xmlchange_exe --id NTASKS_CPL --val 984
$xmlchange_exe --id NTHRDS_CPL --val 4
$xmlchange_exe --id ROOTPE_CPL --val 384

$xmlchange_exe --id NTASKS_GLC --val 1
$xmlchange_exe --id NTHRDS_GLC --val 1
$xmlchange_exe --id ROOTPE_GLC --val 384

$xmlchange_exe --id NTASKS_ROF --val 984
$xmlchange_exe --id NTHRDS_ROF --val 4
$xmlchange_exe --id ROOTPE_ROF --val 384

$xmlchange_exe --id NTASKS_WAV --val 384
$xmlchange_exe --id NTHRDS_WAV --val 4
$xmlchange_exe --id ROOTPE_WAV --val 0

end

[cameronsmith1]

Issue E: I agree with @PeterCaldwell , that the easiest solution is to delete the error test. Fortunately, if the user makes the mistake of requesting more time than the debug queue allows, they will find out instantly when the job gets submitted, so the risk is minor.

[cameronsmith1]

Issue B: @mfdeakin-sandia , I am not sure what it means to 'run interactively' in this context? Can you clarify?

BTW, sometimes I get prompted by CIME to say whether I wanted to use or replace the bld/run directories. I am still learning about this new feature. Is this what you are referring to?

[cameronsmith1]

Issue E: For the problem with the lack of the debug queue on skybridge, how about we set debug_queue=true by default, and then in the run_acme section setting queue properties, have the skybridge section change from debug to the normal queue?

[mfdeakin-sandia]

Issue B: By run interactively, I mean having a terminal which you can interact with. The prompt from CIME is what I'm referring to, and is related to the incoming feature to CIME to automatically delete old bld and run directories and my premature removal of that feature here.
I should also note that I'm concerned about the script doing something potentially dangerous like circumventing the build system; as I would make mistakes if I did that regularly, and have made similar ones in the past. I think a better solution is necessary, and note that one of the CMDV projects is working on one where if I understand correctly the goal is to create a store where everything is built in various configurations. I don't know when this will be available, so waiting for it probably isn't an option unfortunately.
Issue E and F: I will implement that check and delete those PE layouts.
Issue C: I need to look more into this - I think the issue is that the workflow that CIME encourages and uses by default is different from what you all would prefer? If so, we should develop some consensus on this for things in the ACME repository, as the CIME and run_acme script requiring different workflows seems bad to me.

[cameronsmith1]

Here is the status for each issue raised (IMHO):

A) Solution in PR 1391.
B) Unresolved.
C) Unresolved. Defer?
D) Done.
E) Michael will implement.
F) Michael will implement.

[rljacob]

@mfdeakin-sandia: for "C", do you mean different directory structures?

[mfdeakin-sandia]

@rljacob Yes, the same thing being discussed in issue #1388

[rljacob]

I just want to make sure "C" refers to the directory layout we're discussing here: #1388 (comment)

[cameronsmith1]

Hi @rljacob . Yes, Issue C is essentially the discussion that issue #1388 (comment) has turned into. For the purposes of this PR, it would be good to resolve issue C. However, based on that other discussion, it might take a while to decide on, and implement, the proper solution, in which case I propose that the rest of this PR go ahead, and we will deal with it in a future PR.

To put it another way, if everything else in this PR is done except C, then I think we should push this PR.

[cameronsmith1]

Hi @mfdeakin-sandia , thank you for merging some of the fixes we have discussed. If I understand the merges correctly, you have now implemented the changes for issue F and the first half of issue E.

Still outstanding is the second half of issue E, ie deleting that error test and message for whether the wallclock for the debug queue is 30 min.

Do I have this correct?

[mfdeakin-sandia]

@cameronsmith1 Yes, though I believe I finished E with my last commit

[cameronsmith1]

Here is the updated status for each issue raised (IMHO):

A) Proposed solution in PR #1391.
B) Unresolved.
C) Unresolved. (proposed solution: add --script-root to CIME, see #1388)
D) Done.
E) Done.
F) Done.

[cameronsmith1]

It looks like we are nearly there for this PR (IMHO). I am currently testing the changes.

For issue B, is there a way to suppress the CIME prompts when directories already exist, ie when it asks '(r)eplace, (a)bort, or (u)se existing?' (among other issues, having the user answer these questions loses some of the provenance provided by the run_acme script)

[mfdeakin-sandia]

It doesn't look like it, unfortunately; at least until the next CIME version comes in

[jgfouca]

@cameronsmith1 suppressing interactive prompts will be available upon the very next ESMCI integration.

[cameronsmith1]

OK. In that case I suggest we go with what we have. It is workable.

BTW, I tested the run_acme with the new ne4 PE layout, and it all worked.

Are there any other outstanding issues that need to be addressed in this PR? If not, I suggest we push to master.

[mfdeakin-sandia]

I'm not aware of any; it passed the jenkins test last night, so I think it's good

[cameronsmith1]

There are some unresolved issues, however I think is OK to push this to master and deal with the unresolved issues later.

[cameronsmith1]

Thanks, @mfdeakin-sandia .