Skip to content
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

Quantum Coin: Clicking on Initial State radio buttons doesn't always set the corresponding probability to 1. #28

Open
arouinfar opened this issue Sep 11, 2024 · 9 comments
Assignees
Labels

Comments

@arouinfar
Copy link

This is the default configuration of the quantum coin:
image

Clicking the Down radio button will set P(down)=1, which is the desired behavior.
image

However, if we instead click the Up radio button, which is already selected, nothing happens. The probability of up remains 0.8, as in the first screenshot. We discussed in the 9/4/24 design meeting, and think this is a bug. The desired behavior would be for P(up) to get set to 1, because these radio buttons should put the coin into one of the basis states (P=1).

@arouinfar arouinfar added the type:bug Something isn't working label Sep 11, 2024
@AgustinVallejo
Copy link
Contributor

As said above, the reason is that the radio button thinks it's selected. We need to untoggle the previously selected state if the sliders are used, because it's no longer 100% that coin. Maybe we'll have to move away from radio buttons but do something really similar based on P?

@arouinfar
Copy link
Author

We need to untoggle the previously selected state if the sliders are used, because it's no longer 100% that coin. Maybe we'll have to move away from radio buttons but do something really similar based on P?

I have a wild idea. Could we use a pair of sticky toggle buttons? If the coin state deviates from the basis state, the button could unstick.

The flow could be something like this...

  1. Press P(down) button, and it takes on the "stuck" appearance and sets P(down) = 1.
  2. If P(down) changes, whether because the slider was used or P(up) was pressed, the P(down) button unsticks.

@AgustinVallejo
Copy link
Contributor

Patch for stepping back from RadioButtonGroup:

Subject: [PATCH] Asdf
---
Index: js/coins/view/InitialCoinStateSelectorNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/coins/view/InitialCoinStateSelectorNode.ts b/js/coins/view/InitialCoinStateSelectorNode.ts
--- a/js/coins/view/InitialCoinStateSelectorNode.ts	(revision 044224cbfed5b36ba7df8242e4e1ff133c4b139e)
+++ b/js/coins/view/InitialCoinStateSelectorNode.ts	(date 1726597285662)
@@ -11,9 +11,10 @@
 import NumberProperty from '../../../../axon/js/NumberProperty.js';
 import Property from '../../../../axon/js/Property.js';
 import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
+import { combineOptions } from '../../../../phet-core/js/optionize.js';
 import PhetFont from '../../../../scenery-phet/js/PhetFont.js';
-import { Color, Text, VBox } from '../../../../scenery/js/imports.js';
-import RectangularRadioButtonGroup, { RectangularRadioButtonGroupOptions } from '../../../../sun/js/buttons/RectangularRadioButtonGroup.js';
+import { Color, HBox, Text, VBox } from '../../../../scenery/js/imports.js';
+import RectangularRadioButton, { RectangularRadioButtonOptions } from '../../../../sun/js/buttons/RectangularRadioButton.js';
 import Panel from '../../../../sun/js/Panel.js';
 import Tandem from '../../../../tandem/js/Tandem.js';
 import { SystemType } from '../../common/model/SystemType.js';
@@ -54,51 +55,53 @@
       font: new PhetFont( { size: 18, weight: 'bold' } )
     } );
 
-    const radioButtonGroupOptions: RectangularRadioButtonGroupOptions = {
-      orientation: 'horizontal',
-      spacing: 22,
-      radioButtonOptions: {
+    const radioButtonOptions = {
         xMargin: 4,
         yMargin: 4,
         baseColor: Color.WHITE
-      },
-      tandem: tandem.createTandem( 'initialOrientationRadioButtonGroup' )
-    };
+      };
 
-    let initialOrientationRadioButtonGroup;
+    let initialCoinStateItems;
     if ( systemType === 'classical' ) {
-      const initialCoinStateItems = ClassicalCoinStateValues.map( stateValue => ( {
-        createNode: () => new ClassicalCoinNode(
+      initialCoinStateItems = ClassicalCoinStateValues.map( stateValue => {
+        const coinNode = new ClassicalCoinNode(
           new Property<ClassicalCoinStates>( stateValue ),
           RADIO_BUTTON_COIN_NODE_RADIUS,
           Tandem.OPT_OUT
-        ),
-        value: stateValue,
-        tandemName: `${stateValue.toLowerCase()}RadioButton`
-      } ) );
-      initialOrientationRadioButtonGroup = new RectangularRadioButtonGroup<ClassicalCoinStates>(
-        initialCoinStateProperty as Property<ClassicalCoinStates>,
-        initialCoinStateItems,
-        radioButtonGroupOptions
-      );
+        );
+        return new RectangularRadioButton(
+          initialCoinStateProperty as Property<ClassicalCoinStates>,
+          stateValue,
+          combineOptions<RectangularRadioButtonOptions>( {
+            content: coinNode,
+            tandem: tandem.createTandem( `${stateValue.toLowerCase()}RadioButton` )
+          }, radioButtonOptions )
+        );
+      } );
     }
     else {
-      const initialCoinStateItems = QuantumCoinStateValues.map( stateValue => ( {
-        createNode: () => new QuantumCoinNode(
+      initialCoinStateItems = QuantumCoinStateValues.map( stateValue => {
+        const coinNode = new QuantumCoinNode(
           new Property<QuantumCoinStates>( stateValue ),
           new NumberProperty( stateValue === 'up' ? 1 : 0 ),
           RADIO_BUTTON_COIN_NODE_RADIUS,
           Tandem.OPT_OUT
-        ),
-        value: stateValue,
-        tandemName: `${stateValue.toLowerCase()}RadioButton`
-      } ) );
-      initialOrientationRadioButtonGroup = new RectangularRadioButtonGroup<QuantumCoinStates>(
-        initialCoinStateProperty as Property<QuantumCoinStates>,
-        initialCoinStateItems,
-        radioButtonGroupOptions
-      );
+        );
+        return new RectangularRadioButton(
+          initialCoinStateProperty as Property<QuantumCoinStates>,
+          stateValue,
+          combineOptions<RectangularRadioButtonOptions>( {
+            content: coinNode,
+            tandem: tandem.createTandem( `${stateValue.toLowerCase()}RadioButton` )
+          }, radioButtonOptions )
+        );
+      } );
     }
+
+    const initialOrientationRadioButtonGroup = new HBox( {
+      children: initialCoinStateItems,
+      spacing: 22
+    } );
 
     const selectorPanelContent = new VBox( {
       children: [ selectionPanelTitle, initialOrientationRadioButtonGroup ],

AgustinVallejo added a commit that referenced this issue Sep 17, 2024
@AgustinVallejo
Copy link
Contributor

This is done and is currently as discussed with @jbphet and @arouinfar. However, I had some comments regarding this, mainly:

  1. Currently when the probability is not 0 nor 1, both buttons go deselected. But from a physics standpoint, shouldn't they actually be BOTH selected? Meaning the prepared coin is in the superposed state of both basis at the same time.
  2. A possible way to address the above is to have another basis state that shows the mixed superposition, shown in the image and patch below:

image

Subject: [PATCH] Third basis state
---
Index: js/coins/model/CoinsExperimentSceneModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/coins/model/CoinsExperimentSceneModel.ts b/js/coins/model/CoinsExperimentSceneModel.ts
--- a/js/coins/model/CoinsExperimentSceneModel.ts	(revision b152d13e51e971e152a6b370cb47402f6806bdcf)
+++ b/js/coins/model/CoinsExperimentSceneModel.ts	(date 1726605369986)
@@ -71,7 +71,8 @@
     } );
     this.stateBiasProperty = new NumberProperty( options.initialBias, {
       range: new Range( 0, 1 ),
-      tandem: options.tandem.createTandem( 'stateBiasProperty' )
+      tandem: options.tandem.createTandem( 'stateBiasProperty' ),
+      reentrant: true
     } );
     const singleCoinTandem = options.tandem.createTandem( 'singleCoin' );
     if ( options.systemType === 'classical' ) {
@@ -135,9 +136,7 @@
     // If this is a quantum system, changing the initial state of the coin sets the bias to match that coin.
     if ( this.systemType === 'quantum' ) {
       this.initialCoinStateProperty.lazyLink( initialCoinState => {
-        if ( initialCoinState !== 'superposed' ) {
-          this.stateBiasProperty.value = initialCoinState === 'up' ? 1 : 0;
-        }
+        this.stateBiasProperty.value = initialCoinState === 'up' ? 1 : initialCoinState === 'superposed' ? 0.5 : 0;
       } );
 
       this.stateBiasProperty.lazyLink( bias => {
@@ -145,7 +144,6 @@
           this.initialCoinStateProperty.value = 'superposed';
         }
         else {
-          // TODO: Wouldn't this cause a reentry? https://github.com/phetsims/quantum-measurement/issues/28
           this.initialCoinStateProperty.value = bias === 1 ? 'up' : 'down';
         }
       } );
Index: js/coins/view/InitialCoinStateSelectorNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/coins/view/InitialCoinStateSelectorNode.ts b/js/coins/view/InitialCoinStateSelectorNode.ts
--- a/js/coins/view/InitialCoinStateSelectorNode.ts	(revision b152d13e51e971e152a6b370cb47402f6806bdcf)
+++ b/js/coins/view/InitialCoinStateSelectorNode.ts	(date 1726605225855)
@@ -21,7 +21,7 @@
 import quantumMeasurement from '../../quantumMeasurement.js';
 import QuantumMeasurementStrings from '../../QuantumMeasurementStrings.js';
 import { ClassicalCoinStates, ClassicalCoinStateValues } from '../model/ClassicalCoinStates.js';
-import { QuantumCoinStates, QuantumCoinStateValues, QuantumUncollapsedCoinStates } from '../model/QuantumCoinStates.js';
+import { QuantumCoinStates, QuantumCoinStateValues, QuantumUncollapsedCoinStates, QuantumUncollapsedCoinStateValues } from '../model/QuantumCoinStates.js';
 import ClassicalCoinNode from './ClassicalCoinNode.js';
 import CoinNode from './CoinNode.js';
 import QuantumCoinNode from './QuantumCoinNode.js';
@@ -83,10 +83,10 @@
       } );
     }
     else {
-      initialCoinStateItems = QuantumCoinStateValues.map( stateValue => {
+      initialCoinStateItems = QuantumUncollapsedCoinStateValues.map( stateValue => {
         return createCoinRadioButton( stateValue, new QuantumCoinNode(
-          new Property<QuantumCoinStates>( stateValue ),
-          new NumberProperty( stateValue === 'up' ? 1 : 0 ),
+          new Property<QuantumCoinStates>( stateValue === 'superposed' ? 'up' : stateValue ),
+          new NumberProperty( stateValue === 'superposed' ? 0.5 : stateValue === 'up' ? 1 : 0 ),
           RADIO_BUTTON_COIN_NODE_RADIUS,
           Tandem.OPT_OUT
         ) );
Index: js/coins/model/QuantumCoinStates.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/coins/model/QuantumCoinStates.ts b/js/coins/model/QuantumCoinStates.ts
--- a/js/coins/model/QuantumCoinStates.ts	(revision b152d13e51e971e152a6b370cb47402f6806bdcf)
+++ b/js/coins/model/QuantumCoinStates.ts	(date 1726605249974)
@@ -11,5 +11,5 @@
 export type QuantumCoinStates = ( typeof QuantumCoinStateValues )[number];
 
 // Uncollapsed states when preparing the coin
-export const QuantumUncollapsedCoinStateValues = [ 'up', 'down', 'superposed' ] as const;
+export const QuantumUncollapsedCoinStateValues = [ 'up', 'superposed', 'down' ] as const;
 export type QuantumUncollapsedCoinStates = ( typeof QuantumUncollapsedCoinStateValues )[number];
\ No newline at end of file

@arouinfar
Copy link
Author

@AgustinVallejo thanks for working on this. We'll discuss more during design meeting tomorrow.

Currently when the probability is not 0 nor 1, both buttons go deselected. But from a physics standpoint, shouldn't they actually be BOTH selected? Meaning the prepared coin is in the superposed state of both basis at the same time.

No, both should not be selected. The |↑> and |↓> states are orthogonal and form a basis set, meaning the states are linearly independent and cannot be expressed as superposition. Pressing these buttons should put the coin into the respective basis state. Seeing the button become deselected when in a superposition serves the learning goals.

A possible way to address the above is to have another basis state that shows the mixed superposition, shown in the image and patch below:

We cannot do this. A mixed state is a superposition of basis states. By definition, a basis state cannot be expressed as a superposition.

@arouinfar
Copy link
Author

In the 9/10/24 design meeting, we reviewed @AgustinVallejo's patch #28 (comment) that uses two individual radio buttons. Overall, the team liked this behavior and wants to proceed with these changes.

It was a bit unclear what implications there would be for the PhET-iO API, so once things are hooked up, I'd like to review the tree as it relates to this component.

Back to @AgustinVallejo and @jbphet.

@AgustinVallejo
Copy link
Contributor

Three is now working, please take a look @arouinfar

Regarding this component, the radio buttons are not under a group, which is a bit unusual, but it works. Let us know what you think

@AgustinVallejo
Copy link
Contributor

In today's meeting it was decided to add a top level RadioButtonGroup for the buttons, or an explicit panel to toggle the visibility of the whole grey box.

@AgustinVallejo
Copy link
Contributor

AgustinVallejo commented Sep 27, 2024

I believe this whole feature is ready to review, back to @arouinfar

Also, heads up: The quantum initial state property lists "up", "down" and "superposed"... @jbphet and I are looking into maybe creating a better behavior.

@AgustinVallejo AgustinVallejo removed their assignment Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants