-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes to (_internal_)create_program() #327
Changes to (_internal_)create_program() #327
Conversation
…ents Include measurements defined on subtemplates of multi channel templates
…ents Fix parameter_names and measurement_names
…encePT, ForLoopPT, AtomicPT (and thus all subclasses of that) + tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good beside some stuff that might be to slow in some cases. But we can tackle this if it becomes a problem.
qctoolkit/pulses/pulse_template.py
Outdated
@@ -118,7 +118,7 @@ def create_program(self, *, | |||
if parameters is None: | |||
parameters = dict() | |||
if measurement_mapping is None: | |||
measurement_mapping = dict() | |||
measurement_mapping = dict() # todo (2018-08-02): should not be empty but an identity mapping; requires PT to be a MeasurementDefiner (?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
measurement_names
is an abstract property of PulseTemplate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uh... done :D
measurement_parameters = {parameter_name: parameters[parameter_name].get_value() | ||
for parameter_name in self.measurement_parameters} | ||
except KeyError as e: | ||
raise ParameterNotProvidedException(str(e)) from e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you create an exta dictionary for the measurement parameters? They should be a subset of parameter_names
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be but currently aren't according to #230
measurements = self.get_measurement_windows(measurement_parameters, measurement_mapping) | ||
if self.duration.evaluate_numeric(**duration_parameters) > 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be a better way. But this is an optimization for the future.
… additional test for ForLoopPT._internal_create_program()
…easurement_mapping by replacing them with an identity mapping for all measurement names
Implementations of new signature PT._internal_create_program for RepetitionPT, SequencePT, ForLoopPT, AtomicPT (and thus all subclasses of that) + tests.
MappingPT is the only one missing now.
After that is done, #287 should be finished.