Conversation
Krastanov
left a comment
Krastanov
left a comment
There was a problem hiding this comment.
Thank you! This is a great start. I left in a few comments on the design. We can also discuss them at the meeting tomorrow if that would be of use.
| uint8_t rc = redch(c); | ||
| uint8_t bc = bluech(c); | ||
| uint8_t gc = greench(c); | ||
| uint8_t bc = bluech(c); |
There was a problem hiding this comment.
formatting and reordering comment: if moving this is not necessary, please avoid doing it in a "feature" PR, so that git-blame is easy to use in the future.
I also generally prefer the order to be the standard RGB.
| } // end namespace | ||
|
|
||
| #endif // QBEAD_H | ||
| #endif // QBEAD_H No newline at end of file |
There was a problem hiding this comment.
avoid these end-of-file modifications
| float T_imu; // last update from the IMU | ||
|
|
||
| float t_ble, p_ble; // theta and phi | ||
| State state; |
There was a problem hiding this comment.
We do not need state to be member of bead. In particular, when we start looking into working with entangled states or mixed states, it would be much easier if they are decoupled
There was a problem hiding this comment.
Hello, thank you for your feedback. However, I dont fully understand, so the state object should not be part of the qbead object? I was thinking that each instance of qbead should include a state, that will manage all the state related functionality. As I understand now, state and qbead should be independent objects, and if a user wants to assign a state to a qbead, first a state obj should be created and then assigned to the qbead? but then both the qbead and the state will hold values of x,y,z,theta,phy, where the values associated to qbead are the once from imu or ble and the values of state are custom assigned?
There was a problem hiding this comment.
is it not possible to keep the state as an instance of qbead, and then if a user wants to assigned the values from the imu or ble to the state, do smth like this: eg. bead.state.setXYZ(bead.x, bead.y, bead.z) ?
There was a problem hiding this comment.
Like (as an example IMU_reader) now:
void loop() {
bead.readIMU();
bead.clear();
bead.setBloch_deg_smooth(bead.state.getTheta(), bead.state.getPhi(), color(255, 0, 255));
bead.show();
delay(5000);
}
be something like:
void loop() {
bead.readIMU();
bead.state.setXYZ(bead.x, bead.y, bead.z)
bead.clear();
bead.setBloch_deg_smooth(bead.state.getTheta(), bead.state.getPhi(), color(255, 0, 255));
bead.show();
delay(5000);
}
then all the imu and ble readouts are stored in qbead and assigned to state when needed?
There was a problem hiding this comment.
The issue is when we have more than one qbeads and want to have interesting entangled states.
Is the "state" simply the state of the LEDs? If that is the case, it really should not be a separate objects to begin with and having it tightly coupled as you do is indeed the most reasonable thing to do.
Is the "state" an object that represents a quantum state? Such an object can be plotted on the "output device" that is the LEDs of the qbead, but it is not the state of the LEDs itself. And we need a quantum state object in order to introduce the abstraction of gates and entanglement -- we do not need an LED state object. If we are going for this separation, then it makes more sense to be explicit, because it will not be always the case that there is one-to-one mapping of state-to-qbead -- we might want to have entangled states on multiple qbeads for instance.
|
in this commit I've added/changed the code as per feedback. Now the qbead class stores x,y,z,t,p from imu and ble. Now these values are not directly assigned to the state class. In examples, now, first it assigns the values to the state eg. : However, I kept the state as an instance of the qbead object. |
Krastanov
left a comment
Krastanov
left a comment
There was a problem hiding this comment.
Thanks, this looks mostly good. I answered what I think was the main point of disagreement. Let me know if this sounds like a reasonable approach.
Given that your formal MS project is over, no pressure on finishing this. When I have time, I will try to complete it as well. Message here if you happen to start working on it to avoid duplicated effort.
| if (theta < 0 || theta > 180 || phi < 0 || phi > 360) { | ||
| return; | ||
| } | ||
| float theta_section = theta / theta_quant; |
There was a problem hiding this comment.
This is a bit weird. Why is this change made? This does not use the input function parameter anymore.
This can probably look better as a new function setBloch_deg(State state)
| float T_imu; // last update from the IMU | ||
|
|
||
| float t_ble, p_ble; // theta and phi | ||
| State state; |
There was a problem hiding this comment.
The issue is when we have more than one qbeads and want to have interesting entangled states.
Is the "state" simply the state of the LEDs? If that is the case, it really should not be a separate objects to begin with and having it tightly coupled as you do is indeed the most reasonable thing to do.
Is the "state" an object that represents a quantum state? Such an object can be plotted on the "output device" that is the LEDs of the qbead, but it is not the state of the LEDs itself. And we need a quantum state object in order to introduce the abstraction of gates and entanglement -- we do not need an LED state object. If we are going for this separation, then it makes more sense to be explicit, because it will not be always the case that there is one-to-one mapping of state-to-qbead -- we might want to have entangled states on multiple qbeads for instance.
2 participants
Thank you for reviewing the previous pr.
In this pr the state class is introduced and integrated into the qbead class, minor changes to .ino files were done to make them compatible with new implementation.
Examples compile, IMU_reader was uploaded to a physical qbead and worked well. More examples are coming soon.