Skip to content

Refactor LockedAmplitude and ShapedAmplitude#110

Merged
goerz merged 4 commits intomasterfrom
refactor-amplitudes
May 2, 2026
Merged

Refactor LockedAmplitude and ShapedAmplitude#110
goerz merged 4 commits intomasterfrom
refactor-amplitudes

Conversation

@goerz
Copy link
Copy Markdown
Member

@goerz goerz commented May 2, 2026

Both of these amplitudes could be made more flexible by getting rid of the type hierarchy that distinguished between, e.g., ShapedContinousAmplitude and ShapedPulseAmplitude. The shape and control can now be vectors or functions independently of each other. This enables substituting optimized pulses into existing amplitudes, among other benefits.

Both of these amplitudes could be made more flexible by getting rid of
the type hierarchy that distinguished between, e.g.,
`ShapedContinousAmplitude` and `ShapedPulseAmplitude`. The shape and
control can now be vectors or functions independently of each other.
This enables substituting optimized pulses into existing amplitudes,
among other benefits.
@goerz goerz added the enhancement New feature or request label May 2, 2026
@goerz goerz requested a review from Copilot May 2, 2026 20:28
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the amplitude layer to remove the old Locked* / Shaped* subtype split and instead represent amplitudes as parametric wrappers whose shape and control can independently be callables or discretized vectors. In the broader codebase, this sits at the control/amplitude boundary used by generator evaluation and propagation.

Changes:

  • Replaces the previous LockedAmplitude and ShapedAmplitude type hierarchies with parametric structs and updated evaluate/substitute methods.
  • Adds support for mixed ShapedAmplitude forms where only the control or only the shape is vector-backed.
  • Expands amplitude tests to cover mixed shape/control combinations and substitution of a callable control with a discretized vector.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
src/amplitudes.jl Refactors amplitude representations, constructors, substitution, display, and evaluation dispatch.
test/test_amplitudes.jl Adds regression coverage for mixed ShapedAmplitude modes and substitution behavior.

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

Comment thread test/test_amplitudes.jl Outdated
Comment thread src/amplitudes.jl
Comment thread src/amplitudes.jl Outdated
Comment thread src/amplitudes.jl
Comment thread test/test_amplitudes.jl
@codecov
Copy link
Copy Markdown

codecov Bot commented May 2, 2026

Codecov Report

❌ Patch coverage is 97.61905% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.5%. Comparing base (df3491e) to head (952dd50).
⚠️ Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
src/amplitudes.jl 97.5% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master    #110     +/-   ##
========================================
+ Coverage    90.4%   90.5%   +0.2%     
========================================
  Files          37      37             
  Lines        2615    2661     +46     
========================================
+ Hits         2362    2407     +45     
- Misses        253     254      +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@goerz goerz merged commit 45a992f into master May 2, 2026
7 checks passed
@goerz goerz deleted the refactor-amplitudes branch May 2, 2026 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants