Skip to content

XFEL GUI updates fall-winter 2025-26#1142

Open
dwpaley wants to merge 15 commits intocctbx:masterfrom
dwpaley:xfel_gui_beamtime
Open

XFEL GUI updates fall-winter 2025-26#1142
dwpaley wants to merge 15 commits intocctbx:masterfrom
dwpaley:xfel_gui_beamtime

Conversation

@dwpaley
Copy link
Copy Markdown
Contributor

@dwpaley dwpaley commented Apr 6, 2026

  • More structured input on Trials tab
  • GUI config is selected through env variable CCTBX_XFEL_SETTINGS
  • Removed LCLS config panel from rungroup dialog
  • Configurable path of lock file for MTZ uploader
  • Add timeout when opening database connections

Co-authored-by: Aaron Brewster asbrewster@lbl.gov
Co-authored-by: David Mittan-Moreau dwmoreau@lbl.gov

phyy-nx and others added 15 commits March 26, 2026 18:56
- Hide the phil blob
- Add first two controls, uc and sg
- Add sync code between phil str and controls
- Add Edit Phil dialog
Includes more validation as well, such as the unit cell being compatible
with the space group
- Remove extra ok/cancel
- Add min spots for hitfinding
- Fix rounding in unit cell and roundtripping of phil using working phil
…sses

When the GUI is launched with a custom settings file, command-line tools
run afterward now correctly use the same settings file. The GUI launcher
sets CCTBX_XFEL_SETTINGS environment variable, which inherits to child
processes. Both load_cached_settings() and save_cached_settings() check
this environment variable first, falling back to the default behavior
if not set.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Users set CCTBX_XFEL_SETTINGS environment variable directly to specify
a custom settings file. All tools (GUI and command-line) read this
environment variable. Removed command-line argument parsing for settings
files to keep the implementation simple with a single mode of operation.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add drive.lock_file phil parameter to allow users to specify a lock file
location when the home directory is not writable (e.g., on compute nodes).

Changes:
- Add drive.lock_file phil parameter (default None)
- Modify Locker class to accept configurable path
- Pass lock_file through pydrive2_interface to Locker
- Default behavior unchanged (uses ~/.upload_mtz.lock when None)

This fixes issues on systems where home directories are not writable from
compute nodes, allowing users to specify an alternative writable location.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Guard against empty files list to prevent incorrect behavior when r['files']
is an empty list (all([]) returns True which is not the intended logic).

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@dwpaley
Copy link
Copy Markdown
Contributor Author

dwpaley commented Apr 6, 2026

To help with review, the following captures the differences against the final state of the NERSC beamtime repo for all files touched by this PR. The important changes are the configurable lockfile path and the new mechanism for giving the GUI settings file; other changes are minor bugfixes.

dwpaley@Mac:~/daily/20260313/.dan/cctbx_project
$ for f in `git diff upstream/master --name-only`; do echo; echo; echo $f; git --no-pager diff  smx_beamtime $f; done > all.diff



xfel/command_line/upload_mtz.py
diff --git a/xfel/command_line/upload_mtz.py b/xfel/command_line/upload_mtz.py
index 1ca6b19da4..adcc77f6f7 100644
--- a/xfel/command_line/upload_mtz.py
+++ b/xfel/command_line/upload_mtz.py
@@ -9,7 +9,6 @@ try:
   import fcntl
 except ImportError:
   fcntl = None
-fcntl=None
 
 
 help_message = """
@@ -28,6 +27,11 @@ drive {
     .help = Id string of the destination folder. If the folder url is \
 https://drive.google.com/drive/u/0/folders/1NlJkfL6CMd1NZIl6Duy23i4G1RM9cNH- , \
 then the id is 1NlJkfL6CMd1NZIl6Duy23i4G1RM9cNH- .
+  lock_file = None
+    .type = path
+    .help = Path to lock file for coordinating concurrent uploads. \
+If None, defaults to ~/.upload_mtz.lock. Set this to a writable location \
+if the home directory is not accessible (e.g., on compute nodes).
 }
 input {
   mtz_file = None
@@ -79,10 +83,20 @@ def _get_log_fname(mtz_fname):
 class Locker:
   """ See https://stackoverflow.com/a/60214222
   """
+  def __init__(self, lock_file_path=None):
+    if lock_file_path is None:
+      self.lock_file_path = os.path.expanduser('~/.upload_mtz.lock')
+      self._using_default = True
+    else:
+      self.lock_file_path = lock_file_path
+      self._using_default = False
+
   def __enter__(self):
     try:
-      self.fp = open(os.path.expanduser('~/.upload_mtz.lock'), 'wb')
+      self.fp = open(self.lock_file_path, 'wb')
     except FileNotFoundError:
+      if not self._using_default:
+        raise
       self.fp = None
     if fcntl and self.fp is not None:
       fcntl.flock(self.fp.fileno(), fcntl.LOCK_EX)
@@ -99,7 +113,7 @@ class pydrive2_interface:
   destination folder.
   """
 
-  def __init__(self, cred_file, folder_id):
+  def __init__(self, cred_file, folder_id, lock_file=None):
     try:
       from pydrive2.auth import ServiceAccountCredentials, GoogleAuth
       from pydrive2.drive import GoogleDrive
@@ -112,11 +126,12 @@ class pydrive2_interface:
     )
     self.drive = GoogleDrive(gauth)
     self.top_folder_id = folder_id
+    self.lock_file = lock_file
 
 
 
   def _fetch_or_create_folder(self, fname, parent_id):
-    with Locker():
+    with Locker(self.lock_file):
       query = {
           "q": "'{}' in parents and title='{}'".format(parent_id, fname),
           "supportsTeamDrives": "true",
@@ -210,7 +225,8 @@ def run_with_preparsed(params):
 
   drive = pydrive2_interface(
       params.drive.credential_file,
-      params.drive.shared_folder_id
+      params.drive.shared_folder_id,
+      lock_file=params.drive.lock_file
   )
   folders = [dataset_root, version_str]
   files = [log_path]


xfel/ui/__init__.py
diff --git a/xfel/ui/__init__.py b/xfel/ui/__init__.py
index 0d9e3d5d9d..bef7a70c3e 100644
--- a/xfel/ui/__init__.py
+++ b/xfel/ui/__init__.py
@@ -1,5 +1,5 @@
 from __future__ import absolute_import, division, print_function
-import os, sys
+import os
 from iotbx.phil import parse
 from libtbx.utils import Sorry
 
@@ -148,13 +148,6 @@ db {
 master_phil_scope = parse(master_phil_str + db_phil_str, process_includes=True)
 
 settings_dir = os.path.join(os.path.expanduser('~'), '.cctbx.xfel')
-#if len(sys.argv)==1:
-if True:
-  settings_file = os.path.join(settings_dir, 'settings.phil')
-else:
-  settings_file = os.path.expanduser(sys.argv[1])
-
-print('Load settings from', settings_file)
 
 known_dials_dispatchers = {
   'cctbx.xfel.xtc_process': 'xfel.command_line.xtc_process',
@@ -173,6 +166,13 @@ def load_phil_scope_from_dispatcher(dispatcher):
   return phil_scope
 
 def load_cached_settings(scope=None, extract=True):
+  # Determine which settings file to use
+  settings_file = os.environ.get('CCTBX_XFEL_SETTINGS')
+  if settings_file is None:
+    settings_file = os.path.join(settings_dir, 'settings.phil')
+
+  print('Load settings from', settings_file)
+
   if scope is None:
     scope = master_phil_scope
   if os.path.exists(settings_file):
@@ -192,8 +192,14 @@ def load_cached_settings(scope=None, extract=True):
       return scope
 
 def save_cached_settings(params):
-  if not os.path.exists(settings_dir):
-    os.makedirs(settings_dir)
+  # Save to the file specified by environment or default
+  settings_file = os.environ.get('CCTBX_XFEL_SETTINGS')
+  if settings_file is None:
+    settings_file = os.path.join(settings_dir, 'settings.phil')
+
+  target_dir = os.path.dirname(settings_file)
+  if not os.path.exists(target_dir):
+    os.makedirs(target_dir)
 
   working_phil = master_phil_scope.format(python_object = params)
   diff_phil = master_phil_scope.fetch_diff(source = working_phil)


xfel/ui/command_line/xfel_gui_launch.py
diff --git a/xfel/ui/command_line/xfel_gui_launch.py b/xfel/ui/command_line/xfel_gui_launch.py
index 36135ec498..a874e012df 100644
--- a/xfel/ui/command_line/xfel_gui_launch.py
+++ b/xfel/ui/command_line/xfel_gui_launch.py
@@ -72,6 +72,7 @@ Configuration options:
 """)
     parse(master_phil_str, process_includes = True).show(attributes_level=2)
     return
+
   app = MainApp(0)
   app.MainLoop()
 


xfel/ui/components/submission_tracker.py


xfel/ui/components/xfel_gui_dialogs.py
diff --git a/xfel/ui/components/xfel_gui_dialogs.py b/xfel/ui/components/xfel_gui_dialogs.py
index 5cf11a54e6..59a59ee40c 100644
--- a/xfel/ui/components/xfel_gui_dialogs.py
+++ b/xfel/ui/components/xfel_gui_dialogs.py
@@ -3058,8 +3058,9 @@ class TrialDialog(BaseDialog):
     edit_phil_dlg.Fit()
 
     if edit_phil_dlg.ShowModal() == wx.ID_OK:
-      self.parse_trial_phil(edit_phil_dlg.phil_box.GetValue())
-      self.sync_controls()
+      _, msg = self.parse_trial_phil(edit_phil_dlg.phil_box.GetValue())
+      if msg is None:
+        self.sync_controls()
 
   def parse_trial_phil(self, target_phil_str):
     # Parameter validation


xfel/ui/db/__init__.py


xfel/ui/db/job.py
diff --git a/xfel/ui/db/job.py b/xfel/ui/db/job.py
index 999d171830..2c164c0d6c 100644
--- a/xfel/ui/db/job.py
+++ b/xfel/ui/db/job.py
@@ -70,7 +70,7 @@ class Job(db_proxy):
 
   def get_log_path(self):
     run_path = get_run_path(self.app.params.output_folder, self.trial, self.rungroup, self.run)
-    return os.path.join(run_path, "stdout", "out.log") if run_path else None
+    return os.path.join(run_path, "stdout", "log.out") if run_path else None
 
   def submit(self, previous_job = None):
     raise NotImplementedError("Override me!")


xfel/ui/db/xfel_db.py

@dwpaley dwpaley mentioned this pull request Apr 6, 2026
@dwpaley dwpaley requested a review from phyy-nx April 6, 2026 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants