Skip to content

Conversation

@jerjohste
Copy link

  • Addition of new pulse shapes (gaussian and tanh).
  • Addition of a pulse sequence viewer to check sequence code will work
  • Addition of pulse loop transfer feature

Copy link
Member

@MatthieuDartiailh MatthieuDartiailh left a comment

Choose a reason for hiding this comment

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

I gave this a quick read. May try to make a more thorough review later

@@ -0,0 +1,86 @@
# -*- coding: utf-8 -*-
# -----------------------------------------------------------------------------
# Copyright 2015-2018 by ExopyPulses Authors, see AUTHORS for more details.
Copy link
Member

Choose a reason for hiding this comment

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

the copyright does not make sense, I would expect to see 2021

"""
#: Amplitude of the pulse this should be a number between -1.0 and 1.0
amplitude = Str('1.0').tag(pref=True, feval=Feval(types=Real))
sigma = Str('10.0').tag(pref=True, feval=Feval(types=Real))
Copy link
Member

Choose a reason for hiding this comment

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

ideally a comment for sigma too

@@ -0,0 +1,88 @@
# -*- coding: utf-8 -*-
# -----------------------------------------------------------------------------
# Copyright 2015-2018 by ExopyPulses Authors, see AUTHORS for more details.
Copy link
Member

Choose a reason for hiding this comment

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

Same as above

"""
#: Amplitude of the pulse this should be a number between -1.0 and 1.0
amplitude = Str('1.0').tag(pref=True, feval=Feval(types=Real))
sigma = Str('10.0').tag(pref=True, feval=Feval(types=Real))
Copy link
Member

Choose a reason for hiding this comment

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

Same as in Gaussian

@@ -0,0 +1,42 @@
# -*- coding: utf-8 -*-
# -----------------------------------------------------------------------------
# Copyright 2015-2018 by ExopyPulses Authors, see AUTHORS for more details.
Copy link
Member

Choose a reason for hiding this comment

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

Same

sequence_vars = Typed(OrderedDict, ()).tag(pref=(ordered_dict_to_pref,
ordered_dict_from_pref))

Copy link
Member

Choose a reason for hiding this comment

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

please strip spaces when saving


channel_num = len(table)
#fig, axs = plt.subplots(4,1, figsize=(15, 8))
fig, axs = plt.subplots(channel_num,1, figsize=(15, 2.5*channel_num),
Copy link
Member

Choose a reason for hiding this comment

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

using constrained_layout makes generally for nicer plots

for k, v in infos.items():
self.write_in_database(k, v)

def compile_and_plot(self, variables):
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure this belongs on the task. I feel like it would be better on the plugin.


database_entries = set_default({'num_loop': 1})

# def check(self, *args, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

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

Why are no checks actually done ?


class TransferPulseLoopTask(InstrumentTask):
"""Build and transfer a pulse sequence to an instrument.
Copy link
Member

Choose a reason for hiding this comment

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

Since this make several several hypothesis on the driver it could be nice to list them.

@rassouly rassouly mentioned this pull request Apr 14, 2021
@MatthieuDartiailh
Copy link
Member

Superseded by #27

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