phetsims / gas-properties

"Gas Properties" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
6 stars 6 forks source link

PhET-iO instrumentation for particles #231

Closed pixelzoom closed 3 weeks ago

pixelzoom commented 2 months ago

Since this work requires fundamental chances to Particle, there is high risk for introducing regressions. For reference, 1.1.0-dev.12 was published on 5/1, before this work was started:

pixelzoom commented 2 months ago

In the above commits, I instrumented the arrays of particles for the particle systems. The state of particles looks pretty good in the State wrapper - positions and velocities are tracked by the downstream sim. But I'm not convinced that this is totally correct -- sometimes the downstream sim seems to deviate. So there's likely more work to do here.

I'd also like to take a stab at factoring a class DiffusionParticleSystem out of DiffusionModel, instead of instrumenting the latter.

pixelzoom commented 2 months ago

In the above commits, I factored DiffusionParticleSytem out of DiffusionModel. This was a big change, but necessary to provide a PhET-iO API that is consistent with the other 3 screens.

For the first 3 screens, we have:

screenshot_3269



And for the Diffusion screen:

screenshot_3268
pixelzoom commented 2 months ago

@zepumph noted that I should consider making Particle implement TPoolable. Since Particle is a base class that cannot be instantiated directly, this would require implementing TPoolable for its subclasses: HeavyParticle, LightParticle, DiffusionParticle1, DiffusionParticle2.

pixelzoom commented 2 months ago

Above I said:

But I'm not convinced that this is totally correct -- sometimes the downstream sim seems to deviate.

Since there is randomness in the sim, it makes sense that there the downstream sim would deviated from the upstream sim when as they continue to run. They will be the same only at the moment when state is set in the downstream sim.

pixelzoom commented 1 month ago

To get the state of all particles for a specific screen, replace {screenTandemName} with the tandem name of the desired screen:

await phetioClient.invokeAsync( 'phetioEngine', 'getPhetioElementState', [ 'gasProperties.{screenTandemName}.model.particleSystem' ] )

For example, for the Ideal screen:

await phetioClient.invokeAsync( 'phetioEngine', 'getPhetioElementState', [ 'gasProperties.idealScreen.model.particleSystem' ] )

In Studio, see IdeaGasLawParticleSystemIO (Ideal, Explore, and Energy screens) and DiffusionParticleSystemIO (Diffusion screen) for the structure of the state object that is returned.

pixelzoom commented 1 month ago

This is looking pretty good. I still have some questions for the PDL team (@samreid and @matthew-blackman) and @zepumph . But in the meantime, here's a dev version:

pixelzoom commented 1 month ago

@arouinfar @Nancy-Salpepi @kathy-phet This is ready for review.

The state object for a single particle is shown below. _previousX and _previousY are not interesting to the client, but are required for collision detection & response. Note that speed and kinetic energy are not included here; they are computed by the histograms as needed.

 {
  mass: number;
  radius: number;
  x: number;
  y: number;
  _previousX: number;
  _previousY: number;
  vx: number;
  vy: number;
};

The state of a particle system is serialized as arrays of particles. In the first 3 screens, model.particleSystem includes these arrays:

heavyParticles: HeavyParticle[];  // heavy particles that are inside the container
lightParticles: LightParticle[]; // light particles that are inside the container
heavyParticlesOutside: HeavyParticle[];  // heavy particles that are outside the container
lightParticlesOutside: LightParticle[]; // light particles that are outside the container

In the Diffusion screen, where particles are always inside the container, model.particleSystem includes these arrays:

particles1: DiffusionParticle1[];
particles2: DiffusionParticle2[];

The follow custom IOTypes were created to support the above serialization. Please review their documentation in Studio.

Since Studio is unable to display the current value for custom IOTypes, none of the above arrays can be inspected in Studio. (Note that this is also true for projectiles in PDL.). To inspect the arrays, it's necessary to use the PhET-iO API. See examples in https://github.com/phetsims/gas-properties/issues/231#issuecomment-2103577265.

Because you can't inspect particle arrays in Studio, I did not feature model.particles. I can imagine featuring them for IOType documentation, but I don't know how useful that is. Let me know if you'd like particleSystem.model to be phetioFeatured: true.

pixelzoom commented 1 month ago

@zepumph noted that I should consider making Particle implement TPoolable.

In 5/13/24 discussion with PDL team, I noted that in the State wrapper, the Downstream sim pauses slightly each time that state is set. PDL exhibits this same behavior.

I asked if the PDL team considered pooling for Projectiles, and @samreid said that they did not optimize for that.

@samreid also did a quick test with GP in the State wrapper. GC doe not appear to be the issue. Most of the time is spent in getState and setState, and very little time spent in the particle constructor. Implementing TPoolable would mean calling an initializer method (like setXY for Vector2) instead of a constructor. Since particle is basically a data structure, there's unlikely to be any significant different between the constructor and an initializer method.

Compared to other things that implement TPoolable (Vector2, Bounds2, Matrix3,...) GP has relatively few instances: 400 particles max in the Diffusion screen, 2000 particles max in the other screens.

So... I'm not going to pursue pooling for particles.

pixelzoom commented 1 month ago

Design meeting 5/20/24 @arouinfar @kathy-phet @Nancy-Salpepi @matthew-blackman @pixelzoom

We decided not to add a type field to particle state, something like:

type: 'heavy' | 'light' | 'diffusion1' | 'diffusion2'

Particles are already separated by type in the PhET-iO state. Adding a type field would be redundant and add unnecessary bloat to what is a large data set.

As @matthew-blackman demonstrated in #244, a wrapper can add type information if needed. For example in particleDataWrapper.html:

        const simulationReadings = {
          'Particle speed': speedForParticle( particleData ),
          'Particle KE': kineticEnergyForParticle( particleData ),
          'Particle type': 'heavy',
          'Stopwatch reading': stopwatchReading
        };
arouinfar commented 1 month ago

Reviewed with @Nancy-Salpepi and the Particle arrays look good in the console and state appears to be restoring correctly. Closing.

arouinfar commented 1 month ago

Reopening, I forgot to respond to @pixelzoom's question about phetioFeatured

Because you can't inspect particle arrays in Studio, I did not feature model.particles. I can imagine featuring them for IOType documentation, but I don't know how useful that is. Let me know if you'd like particleSystem.model to be phetioFeatured: true.

Let's feature model.particleSystem. The IOType Documentation is likely useful, but we'll also be including information about accessing particle data in examples.md. Featuring it in Studio makes it easier for clients to find and explore.

pixelzoom commented 4 weeks ago

model.particleSystem is now featured. Back to @arouinfar for review, close if OK.

arouinfar commented 3 weeks ago

Thanks @pixelzoom, looks good on main. Closing.

phet-dev commented 3 weeks ago

Reopening because there is a TODO marked for this issue.