Finish hklpy2 migration, add 4-ID-G fly scanning, fix startup/logging/SPEC bugs - #76
Open
gfabbris wants to merge 20 commits into
Open
Finish hklpy2 migration, add 4-ID-G fly scanning, fix startup/logging/SPEC bugs#76gfabbris wants to merge 20 commits into
gfabbris wants to merge 20 commits into
Conversation
SpecWriterCallback2 wrote string-valued columns (e.g. eiger_hdf1_full_file_name) inline as a per-row placeholder plus a #U label = value line after every data row. With a string column present this interleaved a #U line between every data row, breaking the contiguous numeric data block that pymca's specfile parser requires, so pymca read the file as having a single column. Filter non-numeric columns (dtype string/array) out of data_labels in descriptor(), and remove the interleaved #U emission in write_scan_data_row(). The result is a clean numeric-only data block; per-point file paths remain available via #MD / #U externals at scan end. Add tests/test_spec_writer.py covering the dropped string column, column-count consistency, and a contiguous data block. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fix bugs found reviewing restore_diffractometer_from_scan (renamed from restore_huber_from_scan): - Update stale __all__ entry that still listed restore_huber_from_scan, which broke `from hkl_utils_hklpy2 import *` (used in _common_startup and id4_raman.startup) with AttributeError at session startup. - Make the "no hklpy2 configuration" ValueError an actual f-string and fix the concatenated "hklpy2configuration" wording. - Replace non-subscriptable info.keys()[0] with a list, and raise a clear error when the user enters a diffractometer name not in the scan. - Extract the duplicated sample-list + overwrite/append prompt into _prompt_clear_mode(); reuse from read_config() and the restore fn. - After restore(), touch the _psi geometry and recompute UB, matching read_config(). Also includes in-progress analyzer/compute_UB updates in the same file. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rename id4_common/utils/hkl_utils_hklpy2.py to hkl_utils.py and update all imports and doc references accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Importing apsbits triggers its import-time configure_logging(), which does
os.makedirs("<cwd>/.logs"). At the beamline the cwd is the read-only DM
experiment directory, so this raised PermissionError and aborted startup
before setup_logging() could redirect to the centralized LOG_PATH.
chdir into a private temp dir while importing apsbits so the throwaway
.logs lands somewhere writable, then clean it up in setup_logging(). Also
harden the LOG_PATH fallback to use a temp dir when cwd is not writable.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Multiple bluesky sessions shared one RotatingFileHandler target (logging.log) under the NFS LOG_PATH. With rotate_on_startup, a new or rotating session renames the file others hold open, staling their handle and producing repeating "OSError: [Errno 116] Stale file handle" tracebacks. pymongo's monitor thread (heartbeat DEBUG every few seconds, reaching the file handler because startup forces root to DEBUG) made it spam constantly. Give each session its own files (logging.<host>.<pid>.log and ipython_logs.<host>.<pid>.log) so rotation is self-contained, and silence pymongo to WARNING via the apsbits `modules` override (explicit level survives the later root=DEBUG). Both changes live in _build_overrides(). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- hkl_utils: rewrite ca() to allow an optional per-call energy override without touching the live beam wavelength, add pa_new() for a full diffractometer/sample orientation summary, add reciprocal lattice and wavelength display to list_reflections/wh, and update eiger beam-center/distance PVs in compute_UB. - polar_diffractometer: reset tth_trans offset when releasing analyzer calibration; higher-precision angle printouts. - local_preprocessors: fix hinted-device lookup for scaler channels in stage_dichro_wrapper. - _local_scan_utils: warn when a detector intends to save images but predict_save_path() returns None, instead of silently dropping it. - logbook_mcr: fetch 5 shift events by default instead of 3. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Trim CLAUDE.md from ~340 to ~135 lines, moving deep-dive material (config/devices.yml patterns, startup flow, device loading/connection internals, data output/logging, docs-site build) into .claude/docs/*.md that get read only when the task at hand touches that area. Un-ignore .claude/docs/ specifically so these are tracked while .claude/settings.local.json stays local-only. Add an "Implementing New Devices & Code" section spelling out the priority order for talking to hardware: existing device signal via oregistry, then apstools/ophyd building blocks, then a new/extended device class, and only as a last resort raw pyepics caget/caput - which must be explicitly flagged when used. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Correct several docs that had drifted from the actual code: - startup-flow: flag that id4_raman/startup.py doesn't share _common_startup.py and has drifted (missing imports, wrong module name for polartools_hklpy2_imports); fix cat/cat_full naming; fix the crl/gslt "core label" example. - configuration: clarify ioc_prefix is illustrative, not a real convention name; reframe DynamicDeviceComponent factories as the mainstream pattern rather than a rare exception; document the third kb_generic.py-style type()-based factory pattern. - device-loading: local_scans.py was split (May 2026, issue #56) into base_scans/move_plans/grid_scans/hkl_scans.py and is now just a compat shim; add apstools_spec_file_writer.py to the callbacks list. - data-output-and-logging: document the tempfile.mkdtemp fallback tier added after the doc was written; correct the temperature-controller claim (the magnet needle valve isn't reachable via temperature_setup). - documentation-site: fix local build instructions to match CI, add the undocumented dev/ deploy case, add missing pages, correct the already-gitignored api/ note. - CLAUDE.md: make the pyepics tech-debt callout self-verifying via grep instead of a hardcoded file list; note E501 is ruff-ignored. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Bring the branch back to main's pre-existing lint baseline:
- polar_diffractometer.py: add the missing docstring on
`AnalyzerDevice.ath_reset_offset` (D102), drop trailing/blank-line
whitespace (W291/W293).
- flyscans.py: remove the unused `AD_prime_plugin2` import (F401; it is
only referenced in comments) and the dead `total_scan_points` local
(F841; `total_images` is what the plan actually uses).
- Run `ruff format` on the six files this branch touched.
- docs/source/{devices_guide,devices_reference}.md: point at
`polar_diffractometer` after the `polar_diffractometer_hklpy2` rename.
Pre-existing issues on main (id4_raman/startup.py import ordering,
peak_position_legacy.py formatting) are left untouched.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The "Code style" job failed on `pre-commit run --all-files`: ruff-format (pinned v0.4.8) collapsed an over-expanded list comprehension in tests/test_spec_writer.py that fits on one line at the 80-col limit. Fixing that alone would have moved the failure to the job's next step, `flake8 src/`, which flags E266 (too many leading '#') on a stray `##` in a commented-out block in hkl_utils.py, introduced on this branch. Normalized it to the single `#` the surrounding lines use. Also shortened an 81-char docstring in the same test file; E501 is in ruff's ignore list so it slipped past lint, but the project style is 80 columns. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Several functions called get_current_run() directly to pull just the run name or start/end times, bypassing the DmException fallback that get_current_run_name() already provided. Introduce get_current_run_info() as the shared fallback (returning name/startTime/endTime) and route all current-run lookups through it or get_current_run_name(). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fix DM-unavailable fallback coverage in dm_utils
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Beamline-driven changes accumulated over the 2026-2 run at 4-ID-G, plus the
fixes they forced. Three main threads:
the HKL user utilities.
position-stream integration).
Closes #70. Closes #71.
1. hklpy2 migration finished
polar_diffractometer_hklpy2.pyis now the realpolar_diffractometer.py; theold
gi/hkl-based implementation is kept aspolar_diffractometer_obsolete.pyfor reference.hkl_utils_hklpy2.pyislikewise renamed to
hkl_utils.py.devices.yml,_common_startup.py,id4_raman/startup.py,utilities.py, and the docs pages follow the rename.Behind the rename:
hklpy2.diffractometer_class_factory(
hkl_soleil/APS POLAR) plus a sharedDiffractometerMixincarrying thetable, area-detector, filter, slit, and analyzer components.
DeferredEpicsMonochromatorRO— hklpy2'sDiffractometerBase.__init__reads
beam.wavelength/beam.energywhile seeding the solver, which with aplain
EpicsSignalROblocks for 60 s and breaksmake_devices(connect=False)when the VDCM IOC is off. The deferred signals return a sane fallback until
EPICS connects, so the two-phase startup still works with a dead IOC.
UBMatrixSignal(_ub_sync) makessample.UBa subscribable ophyd signal,so
Sync_UB_Matrixcan mirror UB (and theh2/k2/l2azimuthal extras andsimulated motor positions) from the main geometry to its
_psicompanion.Kind.config | Kind.normal, andauxiliary_axis_namesdrops nestedPseudoPositionersub-devices — hklpy2'swh(full=True)/pa()callround(component.position, ...), and.anareturns a namedtuple with no
__round__.hkl_utils.pyuser-facing changesread_diffractometer_config_scan()replacesrestore_huber_from_scan(),which was still on the hklpy
run_orientation_info/restore_sample/restore_reflectionspath. It now useshklpy2.run_utils.get_run_orientationand
diffractometer.restore(..., restore_samples=True, restore_extras=True, restore_constraints=True), prompts when the scan holds a differentdiffractometer, and recomputes UB. Closes Restoring hklpy2 setup from scan not working #71.
write_config/read_configrenamed towrite_diffractometer_config_file/read_diffractometer_config_file;both file and scan restore share a new
_prompt_clear_mode()overwrite-vs-append prompt.reset_constraints()fixed — it called_geom_.reset_constraints()/_geom_.show_constraints(), neither of which exists on an hklpy2diffractometer.
pa_new(): POLAR-specific full printout (orienting reflections withtheir wavelengths, real and reciprocal lattice, U and UB, azimuthal
reference, constraints, mono energy/wavelength/d-spacing).
analyzer_get();ca()takes an optionalenergy.Analyzer calibration
AnalyzerDevice.set_energy()now calibrates thetth_transoffset alongsideth,calc()prints to 4 decimals and restores both offsets on release, andthe new
convert_energy_to_tth_pseudo()/ath_reset_offset()support that.The analyzer moved to pseudo-motor (
th_motor,tth_trans) addressing.2. Fly scanning at 4-ID-G
plans/flyscans.py— newflyscan()plan: 2D snake over the two Jenapiezo axes, fast X and slow Y driven from SoftGlue RAM waveforms through
DAC1/DAC2. The module docstring documents the DAC-swing/modulation-summing
geometry and the position-unit convention.
devices/softgluezynq_g_new.py— newSoftGlueZynqdevice for the ISNsetup: UpDown counter, pulse-train generator, per-interferometer trackers,
dual-RAM components, DAC1 mux, threshold trigger, plus the snake-waveform
builders. Registered as
gsgz_isn; coexists with the existingsoftgluezynq_gand reuses thesoftgluezynq_partsblock library.Enable (gate is for pump-probe); new
setup_flyscan_mode();stage_sigspop of
enablemade conditional so re-staging does not raise.pos_stream(PositionStreamDevice) registered indevices.ymlandgiven
CountersMixinwith a no-op plot API so it slots into the countersmachinery without offering fake plot channels.
piezo_jena_setup()inshorts.pyto check/toggle the per-axis modulationinput that gates the FPGA analog drive.
3. Bug fixes
/gdata(Cannot start bluesky from gdata #70). apsbits resolves its log directory to<cwd>/.logsat import time; at the beamline the cwd is the read-only DMexperiment directory, so
os.makedirs()raisedPermissionErrorand abortedthe import before we could redirect it. The apsbits import now runs from a
private temp dir, and the fallback path checks
os.access(cwd, os.W_OK)before choosing
<cwd>/.logs. Closes Cannot start bluesky from gdata #70.RotatingFileHandleron NFS, so one process rotating the file others heldopen produced
[Errno 116] Stale file handle. Logs are now per-session(
logging.<host>.<pid>.log,ipython_logs.<host>.<pid>.log), and pymongo —whose monitor thread emits a heartbeat DEBUG record every few seconds — is
pinned to WARNING.
arrays) were substituted inline and reported on a
#Uline written betweenevery data row, breaking specfile column detection. They are now dropped from
#Lentirely so the data block stays a contiguous numeric matrix. Covered bythe new
tests/test_spec_writer.py(labels,#N/column-count consistency,block contiguity).
te()wrote to the controller device instead of its.setpoint.stage_dichro_wrapperresolved hintedfields via
oregistry.find(name.replace("_", ".")), which does not work forscaler channels; it now goes through
det.channels_name_map.experiment_setup/change_sampledefaultreset_scan_idtoNone(prompt) instead of silently keeping the old scan id.
_setup_file_iowarns when a detector intends to save images butpredict_save_path()returnsNone, instead of silently dropping it fromthe NeXus
externalslink.ringlightremoved from the baseline;MAX_IMAGESfor the Xspress3 loweredto 12000;
fetch_shift_eventsdefault 3 → 5.4. Docs / repo housekeeping
CLAUDE.mdsplit from 326 lines into 136 lines of core guidance plus fiveon-demand detail docs under
.claude/docs/(configuration, startup flow,device loading, data output & logging, documentation site), which
.gitignorenow allows through. A separate commit corrects stale claims that audit turned up.
Testing
pytest— 62 passed (needs a realapstools; the shared conftest stub makesthe new SPEC-writer test error at collection otherwise).
ruff check/ruff format --checkclean on every file this branch touches.Two pre-existing issues on
mainare deliberately untouched: import orderingin
id4_raman/startup.py(startup import order is load-bearing) andformatting in
peak_position_legacy.py.exercised on the real hardware at 4-ID-G; they have no unit coverage.
Known gaps
VortexXspress34.setup_flyscan_mode()is a stub — the docstring describesnum_images/acq_time/hdf_images, but the body only callssetup_external_trigger()and ignores all three arguments. The Eigerequivalent is implemented.
Printing analyzer setup in
pa#47 (analyzer info inpa) — commented on each with the specifics.🤖 Generated with Claude Code