Once you make this a full fledged PR, can you factor these changes into another commit? This is just changing out checkpoint directory structure.

I think this is specific to Wheeler.

Any reason why this isn't module list?

Because for some reason, module list didn't work on the unlimited queue on Wheeler

Is there a slurm env variable that will give you the current time limit?

Unfortunately no :(

Just a general comment: Would we be able to break up this file either into smaller files for the various classes/functions and maybe even separate commits? This is a lot for one commit and it's some dense code to go through.

"in the input file metadata as 'Executable:'"

Is this the guaranteed message on all clusters?

Haven't seen Slurm output anything else, but we can change this if we run into issues

Does it give a useful return status? That should be more robust than text parsing.

Is this standard notation? In bash {0..5} is inclusive.

I got this from Swift where 1..<3 is a half-open (exclusive) range and 1...3 is a closed (inclusive) range. Ranges are often a source of confusion and ambiguity, and I think the Swift syntax is quite clear.

If this follows Swift syntax, maybe add links to Swift in the relevant places so people can go look at that as well.

Where is this? I don't know what the .parent method of a Path does, but this looks like it's looking for a file named Submit.sh in the support/ directory? Oh also, wouldn't this behavior change if you're in PY_DEV_MODE or not because of linking?

Yep it's next to the Schedule.py file in support/. That's were CMake puts the machine-specific submit script template. It works with PY_DEV_MODE because the .parent is purely lexical (doesn't resolve symlinks). I'm open to better ideas how to copy around submit scripts!

Ok this seems fine then

I forget, is the plan to get these defaults (queue, time limit, nodes/cores) from the Machine file? Or are we getting rid of that?

At the moment I got rid of any use of the machine files in this script. Instead, there's only the submit script template (one per cluster), which sets these defaults. As it stands now we could delete the machine files.

Ok I'll test it out and see how it works

Maybe -c? I know this is make syntax, but -c is srun syntax. Not sure which is better.

I don't know either. I don't think there's a consensus among other tools. I got used to -j because of make. We could allow both, if that helps.

Yeah do both

Leave it at 6. Characters are cheap.

Yea they're cheap but become hard to read/count at some point. Try this: just glancing at 0000 you immediately know that it's 4 zeros. But glancing at 00000 you don't, you have to actually count. So I can make it 4 characters if you think we need that many, but anything beyond that I'd like to avoid. (also keep in mind that the checkpoint/segment names are actually not set in stone, I want to make sure that none of the code really relies on a hard-coded directory structure but just takes as input the files that it needs).

OK, but why do you want to count the digits? I don't see a reason that should be a design goal.

If you don't want to discuss the counter format in this PR then don't change it.

I think readable directory names are a design goal. No need for excessive digits in my opinion. More important for segments than for checkpoints, so if you prefer I can keep the checkpoint format unchanged in this PR.

I hadn't actually compared them, so I tried creating a list of CheckpointNNN and CheckpointNNNNNN names from 0 to 19 (I got tired of typing at that point). I actually found the six-digit version to be significantly easier to read, because the string of zeros acts as a separator between "Checkpoint" and the number. It makes it easier to visually locate the first nonzero digit. Presumably the same would hold for other prefixes.

[Continuing from a thread below, but consolidating because the topic is the same]

Three digits give us over 3 years in daily segments. That seems enough for the vast majority of simulations. As mentioned above the number of digits is not set in stone, so if we want to do a 5-year simulation or do hourly segments for some reason or something like that we can always increase the number of digits for that case.

You assume that segments will always be based on a timer. This has not always been true for SpEC, and while it is a goal for SpECTRE I don't think we should assume that we will always achieve it immediately. SpEC segments could be extremely short (minutes, maybe less) because they were based on some event in the simulation that forced a new segment.

Anyway, @markscheel, you're probably the most experienced SpEC-runner here. Do you want to weigh in?

I see what you mean with the zero-padding visually separating the number from the prefix. How about 4 digits for both segments and checkpoints then? Still not excessive, hence ok to type without having to pay too much attention to the number of keystrokes (which I find kind of hard for 6 digits), but surely enough and visually pleasing.

Is this true? I vaguely think I've seen three-character SpEC segment IDs.

I haven't seen those, but that doesn't say much.

Three digits give us over 3 years in daily segments. That seems enough for the vast majority of simulations. As mentioned above the number of digits is not set in stone, so if we want to do a 5-year simulation or do hourly segments for some reason or something like that we can always increase the number of digits for that case.

I've addressed the design comment in the other thread to try to keep the discussion more organized.

The point was trying to make here wasn't about your proposed design, but that I don't think "With SpEC we never needed more than 26*26=676 segments" is true, so you shouldn't claim it no matter what we decide for SpECTRE.

[optional] If we were confident that deleting the file was the correct thing to do, we would just do it here, so telling the user to do so may be giving bad instructions. I'd just leave it at "File already exists. Retry with --force to overwrite.".

Same comment as above.

Does it give a useful return status? That should be more robust than text parsing.

These indents are rendered weird.

- All arguments to this function, including all additional '--params'.
      For example, the additional '--params' can include parameters
      controlling resolution in the input file.     - 'executable_name': Just
      the name of the executable (basename of the       'executable' path).

Same for the rest below.

This is probably some python thing that I don't understand, but everything I can find on the internet says this is adding a backspace character. Why?

The \b escape marker prevents click from rewrapping the following paragraph: https://click.palletsprojects.com/en/8.1.x/documentation/#preventing-rewrapping

Typo in number of dashes. Again 4 lines down.

What if multiple runs are scheduled?

Changed to Returns 'None' if no or multiple runs were scheduled.

Dashes again.