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

PhET-iO instrumentation for particles #231

Closed
4 tasks done
pixelzoom opened this issue May 3, 2024 · 14 comments
Closed
4 tasks done

PhET-iO instrumentation for particles #231

pixelzoom opened this issue May 3, 2024 · 14 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented May 3, 2024

  • Use the same approach as projectile-data-lab, see: Projectile, ProjectileIO, Field, FieldIO. Check in with them to confirm how I apply this pattern.

  • Replace Particle's Vector2 fields (position, previousPositive, velocity) with number x,y fields, so that ParticleIO will be a flat data structure.

  • Add ParticleIO, IdealGasLawParticleSystemIO, and DiffusionParticleSystemlO to handle serialization.

  • Identify other places where Particle serialization is needed.

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
Copy link
Contributor Author

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
Copy link
Contributor Author

pixelzoom commented May 7, 2024

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
Copy link
Contributor Author

pixelzoom commented May 8, 2024

@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
Copy link
Contributor Author

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 added a commit that referenced this issue May 8, 2024
@pixelzoom
Copy link
Contributor Author

pixelzoom commented May 9, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor Author

pixelzoom commented May 14, 2024

@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
Copy link
Contributor Author

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
Copy link
Contributor

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

@arouinfar
Copy link
Contributor

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.

@arouinfar arouinfar reopened this May 31, 2024
pixelzoom added a commit that referenced this issue Jun 10, 2024
@pixelzoom
Copy link
Contributor Author

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

@pixelzoom pixelzoom assigned arouinfar and unassigned pixelzoom Jun 10, 2024
@arouinfar
Copy link
Contributor

Thanks @pixelzoom, looks good on main. Closing.

@phet-dev phet-dev reopened this Jun 11, 2024
@phet-dev
Copy link
Contributor

Reopening because there is a TODO marked for this issue.

pixelzoom added a commit that referenced this issue Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants