Gas particles slow down or speed up when hitting a moving wall

[KatieWoe]

Test device
Dell
Operating System
Win 10
Browser
Chrome
Problem description
If a particle hits a moving wall that is not controlled by the user it will change speed in a way that is not reflected by temperature. To do this the wall must be moving to change volume in order to keep pressure constant. If the wall is moving in the particle speeds up, and if it is moving out it will slow down. However, the temperature will behave as if nothing of the sort happened. When you stop using the temp slider to cause this change in volume the particle will speed up or slow down to match the indicated temperature. This is in the published sim.
Steps to reproduce

1. Go to a screen that allows constant variables
2. Move the wall in fully
3. Add a single particle
4. Change to holding Pressure constant with changing volume
5. Wait until particle is going toward left wall and start adding heat. Keep bucket active the whole time
6. Observe speed right before and after wall collision
7. Observe speed when you let go of heat bucket
8. Repeat process with large volume getting smaller by cooling temp

Visuals

Troubleshooting information:

!!!!! DO NOT EDIT !!!!!
Name: ‪Gas Properties‬
URL: https://phet.colorado.edu/sims/html/gas-properties/latest/gas-properties_en.html
Version: 1.0.0 2019-08-12 21:54:36 UTC
Features missing: touch
Flags: pixelRatioScaling
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/76.0.3809.100 Safari/537.36
Language: en-US
Window: 1536x722
Pixel Ratio: 2.5/1
WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium)
GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium)
Vendor: WebKit (WebKit WebGL)
Vertex: attribs: 16 varying: 30 uniform: 4096
Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32)
Max viewport: 32767x32767
OES_texture_float: true
Dependencies JSON: {}

[pixelzoom]

This is in the published sim.

For posterity, this refers to version 1.0.0.

[pixelzoom]

I was going to hypothesize that the moving wall has non-zero velocity in this situation. But that is not the case. The heat/cool will definitely be applied to particles in the container. But the overall behavior here is indeed odd. Continuing to investigate...

[pixelzoom]

I added some debug code to monitor the particle speed in the scenario shown in #165 (comment). At the end of IdealGasLawModel.stepSystem (after applying heat, stepping particles, and doing collision detection), I added:

      this.particleSystem.heavyParticles.forEach( particle => {
        console.log( 'speed=' + particle.velocity.magnitude );
      } );

The speed behaves as expected. It increases when heat is applied, decreases when cool is applied, and does not change when it hits the stationary or moving wall.

So the perceived speed change must be due to a performance issue - the entire sim is slowing down while the container resize is animating. It's less noticeable on my test platforms (macOS + Chrome, Safari, and Firefox) but I can imagine that it's more noticeable on the reported platform (Win10 + Chrome, see #146).

I will investigate whether performance can be improved.

[pixelzoom]

Performance improves if I remove this line from IdealGasLawModel.compensateForHoldConstant:

395 this.particleSystem.redistributeParticles( containerWidth / previousContainerWidth );

redistributeParticles horizontally redistributes particles in the container when its width changes. When the user is resizing the container, the particles are paused, so there's no noticeably performance issue. But in the case, the particles continue moving.

But I'm surprised that it's a problem with 1 particle. Maybe it's not actually calling this function that's the problem; perhaps we're just on the edge, and any additional call pushes us over.

[pixelzoom]

Looking at the model code, there's not much going on here. But animating the container is resulting in a lot of view changes -- the container and its lid are redrawn each time the container's width changes.

[pixelzoom]

Here's the code in IdealGasLawContainerNode that redraws the container shape:

container.boundsProperty.link( bounds => {

        // Account for wall thickness, so that container walls are drawn around the container's model bounds.
        const viewBounds = modelViewTransform.modelToViewBounds( bounds )
          .dilated( modelViewTransform.modelToViewDeltaX( container.wallThickness / 2 ) );

        // Update the walls, start at top-left, origin at bottom-right. Shape looks like:
        //  __               ___
        // |                    |
        // |                    |
        // |                    |
        // |____________________|
        //
        wallsNode.shape = new Shape()
          .moveTo( viewBounds.minX + viewOpeningLeftInset, viewBounds.minY )
          .lineTo( viewBounds.minX, viewBounds.minY )
          .lineTo( viewBounds.minX, viewBounds.maxY )
          .lineTo( viewBounds.maxX, viewBounds.maxY )
          .lineTo( viewBounds.maxX, viewBounds.minY )
          .lineTo( viewBounds.maxX - viewOpeningRightInset, viewBounds.minY );

So there's a new Shape created each time. @jonathanolson would there be a performance advantage to mutating the same Shape? And do I need to tell the Path (wallsNode) that its associated Shape has changed?

[pixelzoom]

The modelViewTransform calls may also be significant.

modelViewTransform.modelToViewDeltaX( container.wallThickness / 2 ) is a constant, so I can factor that out of the container.boundsProperty listener. Something like:

      // Half the wall thickness, in view coordinates.
      const viewHalfWallThickness = modelViewTransform.modelToViewDeltaX( container.wallThickness / 2 );

[pixelzoom]

modelViewTransform.modelToViewBounds( bounds ).dilated should be changed to dilate, so that an additional Bounds2 instance is not created.

[pixelzoom]

Oh - I know what's happening. It's not a performance issue. The particle looks like it's slowing down because redistributeParticles adjusts (scales) the x position of particles as the container width is changed.

I don't see any way around this. If we don't call redistributeParticles, then if the container has a large decrease in volume on a tilmestep, we can end up with particles that are to the right of (outside) the container. So unfortunately, I think we're stuck with this behavior, and I recommend that we do nothing.

@arouinfar what do you think?

[pixelzoom]

In the above commits, I pushed the minor performance improvements described in #165 (comment) and #165 (comment) to master and 1.0 branches. They are worth doing, but will not impact the behavior reported in this issue.

[arouinfar]

Thanks for reporting @KatieWoe.

Reading through @pixelzoom's investigation/comments, I agree with this:

I don't see any way around this. If we don't call redistributeParticles, then if the container has a large decrease in volume on a tilmestep, we can end up with particles that are to the right of (outside) the container. So unfortunately, I think we're stuck with this behavior, and I recommend that we do nothing.

I think we can go ahead and mark this as type:wontfix, but since there were some changes made in #165 (comment), we should probably leave this open. I'll defer testing to @KatieWoe.

[pixelzoom]

I unassign this until we begin the next RC cycle, where we'll verify that the minor performance improvements didn't break anything. That can be verified by manually resizing the container, and allowing it to automatically resize in "Pressure V" mode.

[pixelzoom]

I've moved the performance improvements to their own issue, #167, for regression testing.

So this issue can now be closed.