search

phetsims / fluid-pressure-and-flow

"Fluid Pressure and Flow" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
8 stars 5 forks source link

evaluate current state of this sim #323

Open pixelzoom opened 5 years ago

pixelzoom commented 5 years ago

Requested by @ariel-phet and @kathy-phet. Take a look at this sim (issues, code, etc.) Provide an estimate on time to complete.

pixelzoom commented 5 years ago

@ariel-phet @kathy-phet Here's my evaluation.

GitHub Issues

There are a lot of open issues. Excluding this issue, there are currently 76 open issues in fluid-pressure-and-flow, plus one in under-pressure. It's difficult to assess the state of the sim until these issues have been triaged. (6-8 hours)

Code

under-pressure code was indeed moved to fluid-pressure-and-flow, see https://github.com/phetsims/fluid-pressure-and-flow/issues/266#issuecomment-511151574. So all of the code lives in the fluid-pressure-and-flow repo.

There are 9302 lines of code (loc). So it's a relatively big sim, between unit-rates (8400 loc) and graphing-lines (10,400 loc) in size. All of the .js files seem to have reasonable sizes - all but 2 files under 300 loc.

The code is ES5, and should be converted to ES6. Recommended to publish a baseline dev version and do this conversion before doing any other work on the sim. (2-4 hours) @pixelzoom edit: This has been done, see #329.

Code documentation looks decent, but needs cleaning up. @pixelzoom edit: On closer inspection, the doc is incomplete and does not meet PhET standards.

There are visibility annotations (@public, @private,...) for most fields and methods, but some are missing. This will need to be brought up to PhET standards. (2-3 hours)
@pixelzoom edit: On closer inspection, there are numerous missing annotations.

There have been many developers involved in this sim. Authors include Vasily Shakhov (Mlearner), Anton Ulyanov (MLearner), Siddhartha Chinthapally (Actual Concepts), Sam Reid, Aaron Davis, Aadish Gupta, and Michael Kauzmann (roughly in that order).

The model code is difficult to evaluate, since I'm not familiar with the domain. Class names appear to make sense (Particle, Pipe, VelocitySensor,...). But unknown how much work is required here until GitHub issues are triaged and work has begun.

The view code needs refactoring/reorganization. For example, in the Under Pressure screen, I'd expect to find classes named FluidDensityAccordionBox and GravityAccordionBox, but no such classes exist, and those accordion boxes are assembled in the ScreenView. (15-20 hours)

PhET-iO

I don't see any obvious architectural problems that might result in PhET-iO problems later. But it's impossible for me to be sure without becoming more familiar with the implementation.

Estimate for 1.0.0

My very rough guess is ~60 hours to get this sim ready for QA testing. Then ~30 hours to shepherd it through the QA process. This assumes that there are no major GitHub issues, the model is reasonable, and there are no design changes.

arouinfar commented 5 years ago

Thanks @pixelzoom.

This assumes that there are no major GitHub issues, the model is reasonable, and there are no design changes.

Aside from typical pixel polishing, the only design change I am aware of is that the handles on the Flow and Water Tower screens should updated to use HandleNode. I'll open an issue for it.

@pixelzoom edit: That issue is #324.

pixelzoom commented 5 years ago

After doing the conversion to ES6 (#329), I see that this sim is not in the shape than I initially thought. There is significant duplication of code, particularly in the model. Documentation is not up to standards. PhET coding conventions have not been followed. There is a lot to be done here.

The number of GitHub issues (81 open, 329 total) is also cause for concern. Without even looking at the open issues, I would predict that any sim with this many issues before publication is in a compromised state.

So... I'm retracting my previous estimate, and I have no idea how long it would take to complete.

ariel-phet commented 5 years ago

@pixelzoom thanks for taking those first steps, obviously it has been a long time since this sim was started, lots has changed, and it has plenty of back history of mixed development styles.

I would still like this sim to move forward at some point, but it seems like more of a project for a dedicated intern, and not the best use of your time. I will discuss more with @kathy-phet

ariel-phet commented 5 years ago

Unfortunately, due to constraints it looks like this sim will continue to sit fallow for the time being.