add log_tuples function #1896

kk1050 · 2022-05-27T09:30:22Z

ARTIQ Pull Request

Description of Changes

add log_tuples function which can write the runid, start_time and path to a file in the data directory

Related Issue

Log runid/start_time/path tuples to a file in the data directory #810

Type of Changes

	Type
✓	✨ New feature

Steps (Choose relevant, delete irrelevant before submitting)

All Pull Requests

Use correct spelling and grammar.
Close/update issues.
Check the copyright situation of your changes and sign off your patches (git commit --signoff, see copyright).

Code Changes

Test your changes or have someone test them. Mention what was tested and how.

Git Logistics

Split your contribution into logically separate changes (git rebase --interactive). Merge/squash/fixup commits that just fix or amend previous commits. Remove unintended changes & cleanup. See tutorial.

Write short & meaningful commit messages. Review each commit for messages (git show). Format:

topic: description. < 50 characters total.
add log_tuples function

Longer description. < 70 characters per line

Licensing

See copyright & licensing for more info.
ARTIQ files that do not contain a license header are copyrighted by M-Labs Limited and are licensed under LGPLv3+.

sbourdeauducq · 2022-05-27T09:31:37Z

artiq/master/worker_impl.py

@@ -342,7 +347,7 @@ def write_results():
                try:
                    exp_inst.analyze()
                    put_completed()
-                finally:
+                finally:   


sbourdeauducq · 2022-05-27T09:32:01Z

artiq/master/worker_impl.py

@@ -286,6 +286,10 @@ def write_results():
            f["run_time"] = run_time
            f["expid"] = pyon.encode(expid)

+    def log_tuples():
+        with open("tuples.txt", "a") as f:
+            f.write("%s %s %s\n" %(rid, start_time, expid["file"]))


use new style format "".format()"

sbourdeauducq · 2022-05-27T09:32:30Z

"tuples" is too generic of a name. https://en.wikipedia.org/wiki/Tuple

sbourdeauducq · 2022-05-27T09:34:43Z

What happens if multiple experiments are executed at the same time? This can happen if they are scheduled in different pipelines. Can't that corrupt the file?
Might be safer/simpler to write to the file from the scheduler where operations are done sequentially.

sbourdeauducq · 2022-05-27T09:36:11Z

artiq/master/worker_impl.py

@@ -286,6 +286,10 @@ def write_results():
            f["run_time"] = run_time
            f["expid"] = pyon.encode(expid)

+    def log_tuples():
+        with open("tuples.txt", "a") as f:


The current directory of the experiment may be subject to change (see discussions in #1747, #1543).
Would be more future-proof to put it where the master writes other files such as last_rid.pyon.

*Chang the file name * Write to the file from the scheduler

sbourdeauducq · 2022-05-31T09:38:17Z

artiq/master/scheduler.py

@@ -122,6 +122,13 @@ def __init__(self, ridc, worker_handlers, notifier, experiment_db):
        self.notifier = notifier
        self.experiment_db = experiment_db

+    def log_tuples(self, rid, expid):


"Tuples" is not a good name in functions either. What about log_submissions?

sbourdeauducq · 2022-05-31T09:39:40Z

artiq/master/scheduler.py

+        self.rid = rid
+        self.expid = expid
+        start_time = time()
+        with open("rid_time_path.txt", "a") as f:


experiment_submissions.log as a default file name would be better. And make that logging optional and the file name configurable via the command line arguments.

Or even .csv with , as separator?

* make the file name configurable

jordens · 2022-06-01T09:40:44Z

artiq/frontend/artiq_master.py

@@ -53,7 +53,10 @@ def get_argparser():
        "--experiment-subdir", default="",
        help=("path to the experiment folder from the repository root "
              "(default: '%(default)s')"))
-
+    group.add_argument(
+        "--log-filename", default="experiment_submissions.csv", 


Please keep disabled by default

jordens · 2022-06-01T09:40:44Z

artiq/frontend/artiq_master.py

@@ -53,7 +53,10 @@ def get_argparser():
        "--experiment-subdir", default="",
        help=("path to the experiment folder from the repository root "
              "(default: '%(default)s')"))
-
+    group.add_argument(
+        "--log-filename", default="experiment_submissions.csv", 


Please keep disabled by default

sbourdeauducq · 2022-06-02T10:03:18Z

artiq/frontend/artiq_master.py

-        help=("set the filename of the experiment subimission "
-            "(default: '%(default)s')"))
+        "--log-filename", default=None, 
+        help=("(optional) set the filename to create the experiment subimission "))


grammar, typo, space at the end

And I don't think we want this in the "repository" options group. Suggest putting it at the top level like --name

sbourdeauducq · 2022-06-02T10:05:32Z

artiq/master/scheduler.py

+        self.rid = rid
+        self.expid = expid


Why write to self?

artiq/master/scheduler.py

sbourdeauducq · 2022-06-02T10:06:08Z

artiq/master/scheduler.py

+        self.rid = rid
+        self.expid = expid
+        start_time = time()
+        if self.log_filename != None:


is not None

sbourdeauducq · 2022-06-02T10:08:35Z

artiq/master/scheduler.py

@@ -135,6 +146,7 @@ def submit(self, expid, priority, due_date, flush, pipeline_name):
            wd, repo_msg = None, None
        run = Run(rid, pipeline_name, wd, expid, priority, due_date, flush,
                  self, repo_msg=repo_msg)
+        self.log_submissions(rid, expid)


log_submission, singular

sbourdeauducq · 2022-06-02T10:09:57Z

artiq/test/test_scheduler.py

@@ -90,7 +90,7 @@ def setUp(self):

    def test_steps(self):
        loop = self.loop
-        scheduler = Scheduler(_RIDCounter(0), dict(), None)
+        scheduler = Scheduler(_RIDCounter(0), dict(), None, "experiment_submissions.csv")


If you're not reading the file in the test, then don't write it and set it to None. You could just use None as default value in the Scheduler's parameter, then it's backward-compatible and you don't need to edit the test and potentially other code.

sbourdeauducq · 2022-06-02T10:17:12Z

artiq/frontend/artiq_master.py

@@ -53,7 +53,9 @@ def get_argparser():
        "--experiment-subdir", default="",
        help=("path to the experiment folder from the repository root "
              "(default: '%(default)s')"))
-
+    group.add_argument(
+        "--log-filename", default=None, 


There are already other "logging" options in the master and this is confusing. --log-submissions?

*Remove the typos and 'self' *Rename the function and argument *Set the variable to None in test file

sbourdeauducq · 2022-06-06T10:42:33Z

Oops, should have renamed the commit

add log_tuples function

bb7fa94

sbourdeauducq reviewed May 27, 2022

View reviewed changes

*Use new style format

f4a8470

*Chang the file name * Write to the file from the scheduler

sbourdeauducq reviewed May 31, 2022

View reviewed changes

* Rename the function and file

cde8430

* make the file name configurable

jordens reviewed Jun 1, 2022

View reviewed changes

Make creating file optional

11803f0

sbourdeauducq reviewed Jun 2, 2022

View reviewed changes

artiq/master/scheduler.py Show resolved Hide resolved

sbourdeauducq reviewed Jun 2, 2022

View reviewed changes

*Put the argument into another options group

6c010b4

*Remove the typos and 'self' *Rename the function and argument *Set the variable to None in test file

sbourdeauducq merged commit 4ddd273 into m-labs:master Jun 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add log_tuples function #1896

add log_tuples function #1896

kk1050 commented May 27, 2022

sbourdeauducq May 27, 2022

sbourdeauducq May 27, 2022 •

edited

Loading

sbourdeauducq commented May 27, 2022

sbourdeauducq commented May 27, 2022

sbourdeauducq May 27, 2022

sbourdeauducq May 31, 2022

sbourdeauducq May 31, 2022

sbourdeauducq May 31, 2022

jordens Jun 1, 2022

jordens Jun 1, 2022

sbourdeauducq Jun 2, 2022

sbourdeauducq Jun 2, 2022 •

edited

Loading

sbourdeauducq Jun 2, 2022 •

edited

Loading

sbourdeauducq Jun 2, 2022

sbourdeauducq Jun 2, 2022

sbourdeauducq Jun 2, 2022

sbourdeauducq Jun 2, 2022

sbourdeauducq commented Jun 6, 2022

add log_tuples function #1896

add log_tuples function #1896

Conversation

kk1050 commented May 27, 2022

ARTIQ Pull Request

Description of Changes

Related Issue

Type of Changes

Steps (Choose relevant, delete irrelevant before submitting)

All Pull Requests

Code Changes

Git Logistics

Licensing

Choose a reason for hiding this comment

sbourdeauducq May 27, 2022 • edited Loading

Choose a reason for hiding this comment

sbourdeauducq commented May 27, 2022

sbourdeauducq commented May 27, 2022

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

sbourdeauducq Jun 2, 2022 • edited Loading

Choose a reason for hiding this comment

sbourdeauducq Jun 2, 2022 • edited Loading

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

sbourdeauducq commented Jun 6, 2022

sbourdeauducq May 27, 2022 •

edited

Loading

sbourdeauducq Jun 2, 2022 •

edited

Loading

sbourdeauducq Jun 2, 2022 •

edited

Loading