Skip to content

Navigation Menu

Sign in
Sign up

Append pid to spectator filename, if form -M - #284

Open
jodavies wants to merge 1 commit into
form-dev:master from
jodavies:spectatorsM
Open

Append pid to spectator filename, if form -M #284
jodavies wants to merge 1 commit into
form-dev:master from
jodavies:spectatorsM

Conversation

@jodavies

@jodavies jodavies commented Jun 19, 2018

Copy link
Copy Markdown
Collaborator

Make spectators nicer to use when running multiple concurrent jobs. We don't have to guarantee unique temp directories with the -M option.

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.02%) to 48.512% when pulling 3c2a749 on jodavies:spectatorsM into e8c9798 on vermaseren:master.

1 similar comment

coveralls commented Jun 19, 2018
edited
Loading

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.02%) to 48.512% when pulling 3c2a749 on jodavies:spectatorsM into e8c9798 on vermaseren:master.

tueda commented Jun 20, 2018

Copy link
Copy Markdown
Collaborator

Indeed this is nice, though you will lose the perfect control of your spectator file names, for example, the file suffix (maybe it does matter, maybe not). The standard(?) solution is putting PID to the spectator file names, e.g., https://github.com/benruijl/forcer/blob/68a30bfb2aec3672a2f674ca8666bfc970105e24/forcer/forcer-manual.h#L131. I'm not sure if ((int)GetPID())%100000 could be always unique on a machine.

Copy link
Copy Markdown
Collaborator Author

Ah, I did not realize that the pid is available inside form. This also suffices. I think in principle ((int)GetPID())%100000 is not guaranteed to be uniqe, but it is what is used for form's other scratch files so I used it here also.

vermaseren commented Jun 20, 2018 via email

Copy link
Copy Markdown
Collaborator
Hi Josh, If you look in the file startup.c in the routine ReserveTempFiles and then look for the variable AM.MultiRun you will see how the -M flag is applied to the tmp files. Cheers Jos
...
On 20 Jun 2018, at 08:24, jodavies ***@***.***> wrote: Ah, I did not realize that the pid is available inside form. This also suffices. I think in principle ((int)GetPID())%100000 is not guaranteed to be uniqe, but it is what is used for form's other scratch files so I used it here also. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#284 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFLxEnEtcNdz8hQkemL_NmvdEYJFG5Vbks5t-eqmgaJpZM4UtbZu>.

Copy link
Copy Markdown
Collaborator Author

Yes, indeed, this is where I found that ((int)GetPID())%100000 was used for this mode. I simply copied the convention also for spectator files.

By the way, is there any reason the user should have complete control over the name of spectator files, in contrast to the name of all other temp files?

vermaseren commented Jun 20, 2018 via email

Copy link
Copy Markdown
Collaborator
The reason is that you could have several spectator files at the same time. In that case you have to know how to address them. Imagine that I were to deal with the bebe and cross topologies in forcer simultaneaously. Either I have to put the ‘fallout’ into the same spaectator, or each in their own (as is done now, although we do not do it simultaneously). I just wanted to leave this possibility of having more than one. Jos
...
On 20 Jun 2018, at 11:14, jodavies ***@***.***> wrote: Yes, indeed, this is where I found that ((int)GetPID())%100000 was used for this mode. I simply copied the convention also for spectator files. By the way, is there any reason the user should have complete control over the name of spectator files, in contrast to the name of all other temp files? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#284 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFLxEp73v17IsTznGsoT8IeD8nY0oW7aks5t-hKQgaJpZM4UtbZu>.

tueda commented Jun 20, 2018
edited
Loading

Copy link
Copy Markdown
Collaborator

I think this is not the reason why the user needs to have the full control of "real" file names. What the user needs is to have an unique name for each spectator expression, not for actual spectator file. In principle FORM could map logical spectator names into real unique file names.

Copy link
Copy Markdown
Collaborator Author

Aside from the user not being able to control the file extension (if that is really important I suppose the PID could be inserted before the extension?) is there any reason not to merge this one?

Given Issue #112 , I would actually think it is not a bad idea to just force -M mode in all cases. Then less can go wrong for users who are not aware of potential pitfalls.

tueda commented Feb 26, 2024

Copy link
Copy Markdown
Collaborator

Aside from the user not being able to control the file extension (if that is really important I suppose the PID could be inserted before the extension?) is there any reason not to merge this one?

I feel it's OK, but not enough to push it forward for me.

I would prefer to keep file extensions, but it needs some work: what if the case like /tmp/abc.123/spectator or on Windows.

In the first place, the root of the problem is why CreateSpectator needs the filename, I think. CreateSpectator <ExprName> should be enough at the user level, and then FORM could automatically assign the next spectator file name on the file system.

Any input from others?

Given Issue #112 , I would actually think it is not a bad idea to just force -M mode in all cases. Then less can go wrong for users who are not aware of potential pitfalls.

Maybe we need another issue focusing on the -M mode.

Copy link
Copy Markdown
Collaborator Author

Right, giving FORM full control over the filenames, such that it can take its "usual precautions" also solves this issue.

tueda commented Feb 26, 2024

Copy link
Copy Markdown
Collaborator

If any member with writing privileges to the repository wants to merge this PR, then I won't oppose it. It is true that the PR is one of the practical solutions that can be implemented easily and simply.

Copy link
Copy Markdown
Collaborator Author

Any further thoughts on this one? I think relying on the user to put the PID in the spectator filename themselves in the form script when necessary is a recipe for problems in the end.

FORM5 could be the time to take user control of the spectator filenames away completely.

tueda commented Nov 27, 2024
edited
Loading

Copy link
Copy Markdown
Collaborator

Maybe no one will complain about this change to append PID to the names of spectator files.

But, a related question: can checkpoints still be recovered when (削除) sort files and/or (削除ここまで) spectator files have the PID appended?

Edit: sort files are not needed to recover, if I understand correctly.

Copy link
Copy Markdown
Collaborator Author

Good question. The sort files can always be called various names depending on whether form sees an "xxx" file already. I will try to test -- I have never used the checkpoint mechanism before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /