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 support for photons. #47

Closed
pixelzoom opened this issue Aug 5, 2024 · 5 comments
Closed

PhET-iO support for photons. #47

pixelzoom opened this issue Aug 5, 2024 · 5 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Aug 5, 2024

Change the serialization of photons to the approach used for particles in gas-properties. See for example HeavyParticle.HeavyParticleIO. This will eliminate the need to use PhetioGroup. Note that there are 4 subclasses of Particle: Electron, Neutron, Proton, and Photon. Photon is the only subclass that has dynamic instances and additional state, so it is the only subclass that requires serialization (PhotonIO).

This will also require changing the model and rendering approach for the collection of photons. In gas-properties, see IdealGasLawParticleSystem and ParticlesNode.

The model will contain public readonly photons: Photons[], which will be serialized as photons: ReferenceArrayIO( Photon.PhotonIO ).

The view will be something like class PhotonsNode extends Sprites. We'll probably want to cache SpriteInstances for each (integer) wavelength.

@pixelzoom pixelzoom self-assigned this Aug 5, 2024
pixelzoom added a commit that referenced this issue Aug 5, 2024
pixelzoom added a commit that referenced this issue Aug 5, 2024
@pixelzoom pixelzoom changed the title Change serialization approach for photons. Change serialization and rendering approach for photons. Aug 5, 2024
pixelzoom added a commit that referenced this issue Aug 5, 2024
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Aug 13, 2024

The view will be something like class PhotonsNode extends Sprites ...

Using Sprites or CanvasNode for photons seems problematic.

The maximum number of photos that exist at any time is only 21. So the performance gain from using Sprites or CanvasNode is probably close to zero.

There will be a big memory footprint increase by using Sprites. There are ~400 unique colors for photons, each of which would require a SpriteImage (a PhotonNode converted to Canvas). Like VisibleColor (see createColorTable), we'd want to cache a SpriteImage for each color, and that's a relatively large cache. CanvasNode would probably not require a cache, but drawing photons using the Canvas API will be more complicated than the current PhotonNode implementation.

So... Big increase in memory and code complexity, for little/no performance gain.

@pixelzoom pixelzoom changed the title Change serialization and rendering approach for photons. Change serialization and rendering approach for photons to support PhET-iO. Aug 20, 2024
@pixelzoom pixelzoom mentioned this issue Jan 3, 2025
14 tasks
pixelzoom added a commit that referenced this issue Jan 13, 2025
pixelzoom added a commit that referenced this issue Jan 14, 2025
pixelzoom added a commit that referenced this issue Jan 14, 2025
pixelzoom added a commit that referenced this issue Jan 14, 2025
@pixelzoom pixelzoom changed the title Change serialization and rendering approach for photons to support PhET-iO. PhET-iO suppport for photons. Jan 16, 2025
@pixelzoom pixelzoom changed the title PhET-iO suppport for photons. PhET-iO support for photons. Jan 16, 2025
pixelzoom added a commit that referenced this issue Jan 18, 2025
pixelzoom added a commit that referenced this issue Jan 18, 2025
pixelzoom added a commit that referenced this issue Jan 18, 2025
pixelzoom added a commit that referenced this issue Jan 18, 2025
pixelzoom added a commit that referenced this issue Jan 18, 2025
pixelzoom added a commit that referenced this issue Jan 18, 2025
pixelzoom added a commit that referenced this issue Jan 18, 2025
pixelzoom added a commit that referenced this issue Jan 18, 2025
pixelzoom added a commit that referenced this issue Jan 18, 2025
pixelzoom added a commit that referenced this issue Jan 18, 2025
pixelzoom added a commit that referenced this issue Jan 21, 2025
pixelzoom added a commit that referenced this issue Jan 21, 2025
pixelzoom added a commit that referenced this issue Jan 21, 2025
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jan 21, 2025

@DianaTavares @arouinfar @Nancy-Salpepi This is ready for review.

The approach in Gas Properties ended up being inappropriate for this sim. That approach involves keeping an array of object instances, then using either CanvasNode or Sprites to render the entire collection whenever the array changes. Photons are too complicated to render using CanvasNode, and there are too many unique photon colors to make Sprites practical. So I resorted to using PhetioGroup to create dynamic Photon elements -- believe it or not, it seems to be the simplest and best-fit approach in this case.

The relevant part of the Studio tree is shown below, with photonGroup being the PhetioGroup. Only photonGroup.countProperty is featured, and that's because it's featured in common code, with no option to un-feature.

As you review this, note that there is a Studio bug that is making it difficult to inspect the tree under photonGroup - see https://github.com/phetsims/studio/issues/322. You can inspect photonGroup.archetype to see what Properties are associated with a dynamic photon element - these Properties handle their own statefulness, and they are documented as "For internal use only." You can also inspect the documentation for PhotonIO (shown below) to see the fields that are not Properties and therefore must be serialized by PhotonIO.

Image

Image

@pixelzoom
Copy link
Contributor Author

@DianaTavares @arouinfar @Nancy-Salpepi You could probably do this review as part of the more general Studio tree review in #100.

@arouinfar
Copy link

We reviewed in #100 and the photons look good for the needs of PhET-iO partners. No changes are necessary.

@pixelzoom not sure if you want to leave this open until https://github.com/phetsims/studio/issues/322 is addressed or not, so back to you.

@pixelzoom
Copy link
Contributor Author

OK to close. I'll check on https://github.com/phetsims/studio/issues/322 when I look for blocking issues before creating the 1.0 release branch.

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

4 participants