Skip to content

Commit

Permalink
Refactor documentation and remove unnecessary reentrant properties in…
Browse files Browse the repository at this point in the history
… angle properties #109
  • Loading branch information
AgustinVallejo committed Feb 28, 2025
1 parent bedcab5 commit bb38ae3
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 16 deletions.
14 changes: 3 additions & 11 deletions js/common/model/AbstractBlochSphere.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,7 @@ export type AbstractBlochSphereOptions = SelfOptions & PickRequired<PhetioObject

export default abstract class AbstractBlochSphere extends PhetioObject {

// REVIEW: Perhaps this is my lack of content understanding, but this documentation seems confusing. The sentence
// starts with "Angles" yet this is not an array. Do multiple angles determine the calculation of the azimuthal angle?
// Maybe clarify what knowledge a developer needs to have about azimuthal angles in implementation-notes.md and keep this
// documentation specific to the Property itself.
// Angles that determine the direction the vector representation is pointing at azimuthal angle, measured along the XY
// plane from the +X axis, goes from 0 to 2*PI in radians.
// Azimuthal angle, measured along the XY plane from the +X axis, goes from 0 to 2*PI in radians.
public readonly azimuthalAngleProperty: NumberProperty;

// Polar angle, measured from the +Z axis, goes from 0 to PI in radians.
Expand All @@ -48,19 +43,16 @@ export default abstract class AbstractBlochSphere extends PhetioObject {

super( options );

// REVIEW: Document why the azimuthal and polar angles are reentrant and how we know that they will not hit an infinite loop
this.azimuthalAngleProperty = new NumberProperty( options.initialAzimuthalAngle, {
range: new Range( 0, 2 * Math.PI ),
tandem: options.tandem.createTandem( 'azimuthalAngleProperty' ),
phetioReadOnly: true,
reentrant: true
phetioReadOnly: true
} );

this.polarAngleProperty = new NumberProperty( options.initialPolarAngle, {
range: new Range( 0, Math.PI ),
tandem: options.tandem.createTandem( 'polarAngleProperty' ),
phetioReadOnly: true,
reentrant: true
phetioReadOnly: true
} );

this.alphaProperty = new NumberProperty( 1, {
Expand Down
6 changes: 4 additions & 2 deletions js/common/model/AveragingCounterNumberProperty.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@ type DetectionCountSample = {

class AveragingCounterNumberProperty extends NumberProperty {

// REVIEW: Some documentation for `totalAveragingPeriod` and `countSamplePeriod` would be helpful.
private readonly totalAveragingPeriod: number;
// Every countSamplePeriod we store a group of counts for that period
// Then, groups within a totalAveragingPeriod are averaged to get the final value
// So the displayed value updates every countSamplePeriod with the average of the last totalAveragingPeriod
private readonly countSamplePeriod: number;
private readonly totalAveragingPeriod: number;

// variables used in the detection rate calculation
private currentDetectionCount = 0;
Expand Down
4 changes: 3 additions & 1 deletion js/common/view/ProbabilityValueControl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ export default class ProbabilityValueControl extends NumberControl {

super( titleStringProperty, probabilityProperty, RANGE, optionize<ProbabilityValueControlOptions, SelfOptions, NumberControlOptions>()( {

// REVIEW: Add documentation describing why you need to create provide your own layoutFunction.
// Creating our own layout function because NumberControl doesn't have a native support for
// title
// < ------|------ >
layoutFunction: ( titleNode, numberDisplay, slider, leftArrowButton, rightArrowButton ) => {
assert && assert( leftArrowButton && rightArrowButton );
return new VBox( {
Expand Down
3 changes: 1 addition & 2 deletions js/spin/model/SternGerlach.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,7 @@ export default class SternGerlach extends PhetioObject {
this.blockingModeProperty = new EnumerationProperty( BlockingMode.NO_BLOCKER, {
tandem: tandem.createTandem( 'blockingModeProperty' ),
phetioReadOnly: true,
phetioFeatured: true,
reentrant: true
phetioFeatured: true
} );

this.isDirectionControllableProperty = new BooleanProperty( false, {
Expand Down

0 comments on commit bb38ae3

Please sign in to comment.