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

Do individual particles need to be instrumented? #279

Closed
pixelzoom opened this issue Mar 15, 2021 · 4 comments
Closed

Do individual particles need to be instrumented? #279

pixelzoom opened this issue Mar 15, 2021 · 4 comments

Comments

@pixelzoom
Copy link
Contributor

From #262 (comment):

  • Does model.shakerParticles need to be instrumented, with a PhetioGroup for each particle?
  • Does model.precipitate need to be instrumented, with a PhetioGroup for each particle?
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Mar 15, 2021

3/11/21 design meeting @arouinfar @kathy-phet @pixelzoom

Re:

  • Does model.shakerParticles need to be instrumented, with a PhetioGroup for each particle?

Yes. Each particle is dispensed individually at random, and contributes to the solution after it has animated past the surface of the solution and dissolved in solution. Due to this randomness, it's important to record and playback each shaker particle.

Re:

-[ ] Does model.precipitate need to be instrumented, with a PhetioGroup for each particle?

Probably not. We couldn't think of a reason to instrument each precipitate particle. They are a qualitative (derived) indicator of the level of saturation, and nothing else is derived from them. In the State Wrapper, it doesn't matter if they are in the exact positions in the downstream sim.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Dec 5, 2022

#279 (comment) indicates that the precipitate particles "probably" do not need to be individually instrumented. But I'm inclined to instrument them because:

(1) "Probably not" is not reassuring.
(2) They are instrumented in the current implementation. Uninstrumenting them will require an entirely new implementation.
(3) There are at most < 1400 particles, and they are not moving. In Natural Selection, there are > 1400 bunnies, they are all moving, and it's not a problem.
(4) If rendering the particles is a performance problem, that will be addressed by optimizing the Canvas implementation, or reimplementing using Sprites in #276.
(5) If serialization is the problem, that must be addressed for Gas Properties, where each individual particle must be instrumented. So I see no benefit in "kicking the can down the road".

@pixelzoom pixelzoom self-assigned this Dec 5, 2022
@pixelzoom
Copy link
Contributor Author

@arouinfar For the reasons listed in #279 (comment), I'm planning to make no changes here, and keep all particles instrumented. Is that OK with you? If so, you can close this issue.

@arouinfar
Copy link
Contributor

@pixelzoom leaving the particles instrumented sounds reasonable. They look fine in the tree, but I also don't plan to feature them. Performance has been fine, even on my phone.

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

2 participants