PR for v0.2 by carlosbornes · Pull Request #24 · nuts-org/simpyson

carlosbornes · 2026-05-05T17:46:51Z

I've tried to draft the v0.2.0, focusing on suggestions from @MSoares98 in #15 and adding csdf support from #16. Also fixes #18.

What's new

SimpCalc and simulate_spectrum() — replaces the old SimpSim class with a cleaner calculator interface. simulate_spectrum() is a easy to use function that auto-estimates spectral width, offset, and carrier frequency from the spin system.

Simpy data container — unified object for FID, spectrum, and xreim data with lazy FID to spectrum conversion (and back) via FFT and automatic ppm axis calculation.

Pulse sequence templates — built-in CPMAS template and support for custom Tcl pulse sequences via CustomPulseSequence.

.csdf file support — read and write CSDM format #16

Breaking changes

SimpSim renamed to SimpCalc. Any code using SimpSim will need updating.

Bug fixes

Time unit conversion error in FID (* 10e3 → * 1e3)
generate_spinsys() double-wrapping on repeated calls
write_simp() wrong parameter names + circular import
add_spectra() crash on FID-only input
Confusing errors on malformed .spe/.fid files

Tests

Added more test

Docs

Renamed and added some more details to the docs

…rum in the function `simulate_spectrum`

Copilot

Pull request overview

This PR drafts the v0.2.0 release by refactoring the simulation interface around a new SimpCalc/simulate_spectrum() API, introducing a unified Simpy data container for SIMPSON outputs, expanding pulse-sequence templating, and adding initial .csdf read support—along with broad test and documentation updates.

Changes:

Introduces SimpCalc + simulate_spectrum() for generating/running SIMPSON inputs with smarter defaults and pulse-sequence templates.
Adds the Simpy container to unify .fid/.spe/.xreim handling with lazy FFT-based conversions and ppm axis computation.
Refactors IO/GUI/docs/tests around the new APIs and adds initial .csdf reading support.

Reviewed changes

Copilot reviewed 31 out of 37 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
tests/test_utils.py	Adds unit tests for isotope lookup helpers, `get_larmor_freq()`, and `add_spectra()`.
tests/test_simulate_spectrum.py	Adds tests for parameter placement and SW/offset/ref auto-estimation behavior.
tests/test_simpy.py	Adds tests for `Simpy` container behavior, lazy conversions, ppm caching, and writing.
tests/test_quadrupolar.py	Tests quadrupolar detection logic and `simulate_spectrum()` operator defaults.
tests/test_io.py	Adds tests for `read_simp()` dispatch and basic read/write round-trips.
tests/test_custom_pulse.py	Tests parameter filtering/forwarding for custom pulse sequences.
tests/test_calculator_improvements.py	Adds validation/dry-run tests for the new calculator.
src/simpyson/utils.py	Refactors isotope handling, adds `add_spectra()`, and adds `simple_spinsys()`.
src/simpyson/templates.py	Replaces old template approach with `PulseSequenceTemplate` + built-ins + custom Tcl wrapper.
src/simpyson/simpy.py	Introduces the new `Simpy` unified data container with lazy FFT conversions and writers.
src/simpyson/isotope_data.json	Fixes key spelling (`NatAbudance` → `NatAbundance`).
src/simpyson/io.py	Replaces `SimpReader` with functional IO, adds `.csdf` reading, and `write_simp()` wrapper.
src/simpyson/gui.py	Updates GUI to use `Simpy`/`read_simp()`, adds selection settings and combine-spectra feature.
src/simpyson/converter.py	Refactors VASP OUTCAR parsing and updates hz/ppm conversion conventions.
src/simpyson/cli.py	Updates CLI to allow passing files to `simpyson gui`.
src/simpyson/calculator.py	Adds `SimpCalc` and `simulate_spectrum()` implementation.
src/simpyson/init.py	Exposes new public API (`Simpy`, `SimpCalc`, `simulate_spectrum`, IO helpers).
README.md	Updates README for v0.2 API and quick-start examples.
pyproject.toml	Bumps version to 0.2.0, updates Python requirement, and adjusts docs deps.
mkdocs.yml	Updates docs navigation to new notebook structure.
examples/scripts/simulate_basic.py	Adds example for `simulate_spectrum()`.
examples/scripts/read_spectrum.py	Adds example for reading and inspecting spectra with `Simpy`.
examples/scripts/build_calculator.py	Adds example for generating SIMPSON input via `SimpCalc`.
examples/calculator/cpmas_example.in	Adds a CPMAS example input file.
docs/installation/install.md	Updates install docs for Python>=3.10 and GUI verification.
docs/index.md	Updates docs landing page to reflect new API/features.
docs/about/contributors.md	Updates contributor/maintainer statement.
CHANGELOG.md	Adds detailed 0.2.0 changelog entry.
.gitignore	Adds `uv.lock` ignore.
.github/workflows/release.yaml	Updates release workflow action versions.
.github/workflows/docs.yaml	Updates docs workflow branches and switches deployment to `mike`.

Comments suppressed due to low confidence (2)

.github/workflows/docs.yaml:31

actions/setup-python@v6 is likely not a valid tag (commonly used major is v5). If the tag doesn’t exist, docs builds/deploys will fail. Pin to a known existing major or a commit SHA.

      - name: Set up Python
        uses: actions/setup-python@v6
        with:
          python-version: "3.12"
          cache: pip
          cache-dependency-path: pyproject.toml

src/simpyson/gui.py:262

In open_files(), file_format is taken from the filename without normalization, and view is only set for exact matches 'spe', 'fid', 'xreim'. If the user opens files with uppercase/mixed-case extensions (e.g. ETHANOL.SPE) or any unexpected extension, view will be undefined and the GUI will raise UnboundLocalError when populating self.files_data. Consider using file_format = ...lower() and handling the else case (warn + skip).

                file_format = filename.split('.')[-1]
                base_name = os.path.basename(filename)

                data = read_simp(filename, format=file_format)

                if file_format == 'spe':
                    view = 'hz'
                elif file_format == 'fid' or file_format == 'xreim':
                    view = 'fid'

                self.files_data[base_name] = {
                    'data': data,
                    'path': filename,
                    'view': view,
                }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

carlosbornes

Few improvements and cleaning

Co-authored-by: Carlos Bornes <carlos.bornes@natur.cuni.cz>

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Carlos Bornes <carlos.bornes@natur.cuni.cz>

Co-authored-by: Carlos Bornes <carlos.bornes@natur.cuni.cz>

…es, docs error, and updated some docs.

…he example 3 docs with the new functions

@jkshenton

…tion - NoPulse template pads offset args to match channel count; skips offset line entirely when offset is zero - SimpCalc counts channels from spinsys and forwards num_channels to template - SimpCalc auto-sets variable_ref = -variable_offset so spe['hz'] returns an absolute frequency axis without manual correction - simulate_spectrum uses Cq²/νL for CT spectral width estimation and fixes offset/ref sign convention - Add gauss_lb support in SimpCalc and write_simp_main - Upgrade simulate_spectrum defaults to rep2000 / 16 gamma angles - Downgrade spurious REF header warning to debug log Tested against @jkshenton tests at https://github.com/jkshenton/spinsim-tests

jkshenton

Looking great! I've suggested a few places things could be improved further, but it's looking very useful - thanks!

- Fix off-by-one in FID time axis, now uses arange (consistent with _compute_fid), not linspace - Fix csdf coordinate units, use .to('Hz').value instead of bare .value - Normalize spinsys at SimpCalc construction via `_normalize_spinsys()` + property setter. `generate_spinsys()` is now a trivial return - Add SW floor for quadrupolar-only spectra, matching shifts - Add Simpy.__repr__ for useful interactive display - Refactor read_simp dispatch to use _EXT_TO_FMT/_READERS dicts and rename format -> fmt - Remove unused CPMAS p1H/pl1H/ph1H parameters - Update tests to match corrected arange time axis convention Co-Authored-By: Kane Shenton <jkshenton@users.noreply.github.com>

jkshenton

Looks good to me! I would just consider removing the machine-specific Claude allow commands from this PR.

jkshenton · 2026-05-19T14:12:39Z

+{
+  "permissions": {
+    "allow": [
+      "Bash(\"C:\\\\Users\\\\cborn\\\\AppData\\\\Roaming\\\\mamba\\\\envs\\\\simpyson-dev\\\\python.exe\" -m pytest tests/ -v --tb=short)",


These look a bit too specific to your machine tbh - I would keep them out of the main codebase

Good point. Removed

…ted bugs Blocking fixes: - read_simp() was called with format= while its keyword was fmt, so every SimpCalc.run(read_output=True) / simulate_spectrum() call crashed with TypeError after SIMPSON finished, and the GUI could not open any file. The keyword is now `format` everywhere, consistent with Simpy.write(). - Simpy.write(format='xreim') wrote a SIMP-header file without the time axis, which read_xreim() could not parse. It now writes the 3-column "time real imag" text that SIMPSON's -xreim flag produces, with a fallback to FID data when no xreim data is present. Bug fixes: - Simpy.write(format='spe') preserves a shifted frequency axis via the REF header (round-trips after add_spectra interpolation) and writes NP as an integer. - Added a csdf writer (Simpy.write(format='csdf')) so CSDM support covers both reading and writing, as advertised. - read_csdf() computed sw from the coordinate span ((N-1)*step); it now uses the full width N*step. - _proton_freq_to_b0() normalises Hz/kHz/GHz/THz strings to MHz instead of passing them through to get_larmor_freq(), which only accepts T/MHz. - GUI open_files(): lowercases extensions, warns and skips unsupported files instead of raising UnboundLocalError, wraps read errors in a dialog, and supports .xreim (new view) and .csdf files. - GUI save dialog now offers xreim and csdf formats. - SimpCalc.run() cleanup no longer deletes pre-existing files that happen to share the output file name. - from_fid()/from_spe() also clear stale xreim data. Cleanup: - generate_main()/run() read output settings only from output_config, removing dead self.parameters fallbacks (those keys are popped in __init__). - Corrected Pulse90/NoPulse tsw docstrings (default is '1e6/sw', not 1e4). - Removed duplicate classifier in pyproject.toml. Tests (13 new, 80 total): - Mocked-SIMPSON test covering the run() read path that previously crashed, plus a cleanup-safety test. - Round-trip tests for xreim, csdf, and REF-shifted spe files. - Unit tests for _proton_freq_to_b0 unit handling. Co-authored-by: Claude <noreply@anthropic.com>

Implements the findings from the SIMPSON-verified review session (WSL, 2026-06-11). The empirical results there established that SIMPSON places +delta at +delta*|nu_L| regardless of the sign of gamma, and that the carrier offset is a rotating-frame frequency that follows the sign of gamma. Negative-gamma fixes (verified empirically against SIMPSON for 13C and 29Si in that session): - hz2ppm()/ppm2hz() now use abs(get_larmor_freq()); the signed value mirrored every negative-gamma (29Si, 15N, 17O, ...) ppm axis. get_larmor_freq() itself stays signed (physical quantity). - simulate_spectrum() spectral-width estimation uses |nu_L|, fixing collapsed/negative SW for negative-gamma nuclei. - simulate_spectrum() auto-centering: variable_offset = sign(gamma) * center_hz, variable_ref = -center_hz always. Positive-gamma behaviour is unchanged. Other findings: - _proton_freq_to_b0() accepts scientific notation ('8e8' -> '800.0MHz'); bare numeric strings follow the numeric rule (>1e6 means Hz). - channels/nuclei extraction regexes use [^\n]+ instead of [\w\s]+, which matched newlines and swallowed following spinsys lines; per-site nucleus detection could pick a token (e.g. 'shift') from the wrong line. - pulse_90 template pads extra channels with '0 0' (SIMPSON requires an rf/phase pair per channel), using the same num_channels mechanism as no_pulse. Verified on 1- and 2-channel systems. - add_spectra() interpolation path: combined sw is N*step, not the coordinate span (N-1)*step. - Documented that zerofill defaults to np (no implicit zero-filling, kept deliberately) and that CPMAS sw needs no commensurability with spin_rate*gamma_angles (SIMPSON samples acq_block at 1/sw; verified). Tests (30 new, 110 total): - tests/test_pr24_regressions.py (25, CI-safe): axis-magnitude convention, offset/ref signs for both gamma signs via a mocked SIMPSON, SW estimation (17O CT MAS + static, 15N 10-ppm floor, 29Si), sci-notation parsing, regex line-leak regression, Pulse90 padding, add_spectra sw invariant. - tests/test_simpson_e2e.py (5, skipped without SIMPSON on PATH): 13C smoke, both-gamma axis convention, 17O CT width/wrap-around, 33S second-order 1/nu_L^2 field scaling (asserts the scaling ratio, not an absolute position). Co-authored-by: Claude <noreply@anthropic.com>

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 56 out of 64 changed files in this pull request and generated 3 comments.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

MSoares98

Review assisted by Claude and Copilot.

Looks okay to merge.

…s with the new version

carlosbornes · 2026-06-15T15:15:39Z

Thanks @jkshenton and @MSoares98. I will merge it, if some errors appear I will fix, otherwise will make the release still today

carlosbornes and others added 10 commits May 5, 2026 17:48

Major changes to the code implmentation

2164c9a

Delete repeated file

8793d93

Lazy implementation for csd files

5bfa6f5

Added mike to have dev and main docs

540dedf

Fixed bug #18

466d184

Did an initial draft for the calculator

1a65310

Implementation of Simpson calculation and ability to run from python

3f412d2

Fixed some problems with referencing and with how to center the spect…

a1b3a54

…rum in the function `simulate_spectrum`

Increase tests, improve changelog, and docs to reflect new functions

4c6416d

Update README and github actions

901329f

carlosbornes requested review from MSoares98, Copilot and danielcunhapereira May 5, 2026 17:47

Copilot started reviewing on behalf of carlosbornes May 5, 2026 17:47 View session

Copilot AI reviewed May 5, 2026

View reviewed changes

Apply suggestions from code review

a1ef551

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

carlosbornes commented May 6, 2026

View reviewed changes

carlosbornes and others added 8 commits May 6, 2026 13:42

Apply suggestions from code review

42daed1

Co-authored-by: Carlos Bornes <carlos.bornes@natur.cuni.cz>

Apply suggestions from code review

a5dfd2b

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Carlos Bornes <carlos.bornes@natur.cuni.cz>

Apply suggestions from code review

d7687db

Co-authored-by: Carlos Bornes <carlos.bornes@natur.cuni.cz>

Fixed ruff lint, problem with hz to ppm convertion, csdmpy dependenci…

71faef3

…es, docs error, and updated some docs.

Remove leftovers from mike to fix docs build

bbab7c3

Another try at fixing docs problems

dd2c40b

Fixed problems with the CPMAS template, added it to docs, and rerun t…

76469c3

…he example 3 docs with the new functions

carlosbornes marked this pull request as draft May 8, 2026 11:34

carlosbornes requested a review from jkshenton May 8, 2026 13:11

jkshenton reviewed May 12, 2026

View reviewed changes

carlosbornes and others added 2 commits May 14, 2026 09:44

Add citation file according to #23 and fix miss match of license

79e0d68

jkshenton approved these changes May 19, 2026

View reviewed changes

carlosbornes and others added 3 commits May 19, 2026 15:47

Delete .claude permissions

8dfe72f

MSoares98 requested a review from Copilot June 15, 2026 11:13

Copilot started reviewing on behalf of MSoares98 June 15, 2026 11:13 View session

carlosbornes marked this pull request as ready for review June 15, 2026 11:25

Copilot AI reviewed Jun 15, 2026

MSoares98 requested a review from Copilot June 15, 2026 11:48

Copilot started reviewing on behalf of MSoares98 June 15, 2026 11:49 View session

Copilot AI reviewed Jun 15, 2026

View reviewed changes

Comment thread src/simpyson/utils.py Outdated

Comment thread src/simpyson/gui.py Outdated

Comment thread src/simpyson/calculator.py Outdated

Apply suggestions from code review

690689f

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

MSoares98 approved these changes Jun 15, 2026

View reviewed changes

carlosbornes added 2 commits June 15, 2026 16:43

Divide the test_pr24 test into several test files. Rerun the notebook…

d036671

…s with the new version

Add test to read_vasp

5c0a2eb

carlosbornes removed the request for review from danielcunhapereira June 15, 2026 15:15

carlosbornes merged commit f2317b1 into main Jun 15, 2026
1 check passed

carlosbornes deleted the v0.2-dev-calc-draft branch July 28, 2026 13:35

Uh oh!

Conversation

carlosbornes commented May 5, 2026

What's new

Breaking changes

Bug fixes

Tests

Docs

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

carlosbornes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jkshenton left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jkshenton left a comment

Choose a reason for hiding this comment

Uh oh!

jkshenton May 19, 2026

Choose a reason for hiding this comment

Uh oh!

carlosbornes May 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MSoares98 left a comment

Choose a reason for hiding this comment

Uh oh!

carlosbornes commented Jun 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants