Skip to content

[Feature]: Leakage suppression objective#207

Merged
andgoldschmidt merged 3 commits into
harmoniqs:mainfrom
martin-klacan:leakage-suppression
Jun 12, 2025
Merged

[Feature]: Leakage suppression objective#207
andgoldschmidt merged 3 commits into
harmoniqs:mainfrom
martin-klacan:leakage-suppression

Conversation

@martin-klacan

Copy link
Copy Markdown
Contributor

This PR fixes issue #203

Implemented LeakageSuppressionObjective function inside quantum_objectives.jl
Created testitem to test this function.

Comment thread Project.toml Outdated
version = "0.7.2"

[deps]
CairoMakie = "13f3f980-e62b-5c42-98c6-ff1f3baf88f0"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We don't want to add a Makie dependence to keep build times fast.

Makie is an extension package of NamedTrajectory and is used by PiccoloPlots, only.

Comment thread Project.toml Outdated
Libdl = "8f399da3-3557-5675-b5ff-fb832c97cbdb"
LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
NamedTrajectories = "538bc3a1-5ab9-4fc3-b776-35ca1e893e08"
Piccolo = "c4671d76-df94-11ed-2057-43d4fd632fad"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a circular dependency (QuantumCollocation is a subpackage of Piccolo)

Comment thread src/quantum_objectives.jl Outdated
using NamedTrajectories
using PiccoloQuantumObjects
using DirectTrajOpt
using QuantumCollocation

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The package we are testing is QuantumCollocation, so we can't be using QuantumCollocation

Local modules can be added, if necessary (take a look at _problem_templates.jl to see how)

Comment thread src/quantum_objectives.jl Outdated

# Dummy system with leakage indices
struct DummySystem end
PiccoloQuantumObjects.EmbeddedOperators.get_leakage_indices(::DummySystem) = [(1,2), (2,3)]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nice

Comment thread src/quantum_objectives.jl Outdated
where `I_leakage` is given by `get_leakage_indices(system)`.
"""
function LeakageSuppressionObjective(system, name::Symbol, traj; norm_type=:fro, kwargs...)
leakage_inds = get_leakage_indices(system)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i like the idea of having system's that know about their leakage subspace, but maybe we can simplify: make leakage indices an argument to the objective.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(instead of passing the system)

Comment thread src/quantum_objectives.jl
Comment thread src/quantum_objectives.jl Outdated
sum_{(i,j) ∈ I_leakage} norm(U[i, j], norm_type)
where `I_leakage` is given by `get_leakage_indices(system)`.
"""
function LeakageSuppressionObjective(system, name::Symbol, traj; norm_type=:fro, kwargs...)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add types to the args for the compiler, where possible.

@andgoldschmidt andgoldschmidt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

adding "request changes"

@martin-klacan

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback @andgoldschmidt I have just pushed the requested changes, let me know if it looks good!

@andgoldschmidt andgoldschmidt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i think we're basically ready here, just some cosmetics

Comment thread src/quantum_objectives.jl Outdated
using DirectTrajOpt
using TestItems

# using PiccoloQuantumObjects.EmbeddedOperators: get_leakage_indices

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

clean up here

Comment thread src/quantum_objectives.jl
@codecov

codecov Bot commented Jun 11, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@martin-klacan

Copy link
Copy Markdown
Contributor Author

@andgoldschmidt I have removed comments as requested. Hope we can close by the deadline in 9 hours :)

@andgoldschmidt

Copy link
Copy Markdown
Member

@martin-klacan it's ok, as long as the PR is submitted, we have this week to polish off any last details. We'll have no trouble meeting that timeline.

@martin-klacan

Copy link
Copy Markdown
Contributor Author

Oh I see, perfect then! Let me know if any further changes are necessary

@andgoldschmidt
andgoldschmidt merged commit eaa5db3 into harmoniqs:main Jun 12, 2025
5 checks passed
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.

2 participants