phetsims / projectile-data-lab

"Projectile Data Lab" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 0 forks source link

Sampling screen x-axis min/max needs updating #241

Closed catherinecarter closed 5 months ago

catherinecarter commented 5 months ago

Now that the default angle for the cannon is 30º, the thumbnail x-viewing window needs updating to accommodate the lower means for each launcher, each sample size.

Example: Launcher 1's thumbnails

image
samreid commented 5 months ago

Thanks @catherinecarter, good idea. @matthew-blackman and I updated in the commit, can you please review and close if all is well?

catherinecarter commented 5 months ago

The center looks like it's shifted a bit to the right. Below is the dev.20 version (left image) and the new version (right image) for launcher 4: image image

All of the launchers seem a bit to the right. Here's launcher 2 with dev.20 on the left, new on the right: image image

samreid commented 5 months ago

I ran 5x experiments on each launcher and averages the averages. Here is the result:

```diff Subject: [PATCH] Adjust sample thumbnail domain axes, see https://github.com/phetsims/projectile-data-lab/issues/241 --- Index: js/sampling/view/SampleSizeThumbnailNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/sampling/view/SampleSizeThumbnailNode.ts b/js/sampling/view/SampleSizeThumbnailNode.ts --- a/js/sampling/view/SampleSizeThumbnailNode.ts (revision 705aa7cd9cadc0a4c5692a328ba8c7e7caa871fc) +++ b/js/sampling/view/SampleSizeThumbnailNode.ts (date 1710281791609) @@ -61,12 +61,12 @@ const THUMBNAIL_DOMAIN = 40; const thumbnailMean = - index === 1 ? 45 : - index === 2 ? 48 : - index === 3 ? 55 : - index === 4 ? 45 : - index === 5 ? 50 : - index === 6 ? 55 : + index === 1 ? 46.38 : + index === 2 ? 49.72 : + index === 3 ? 55.455 : + index === 4 ? 46.58 : + index === 5 ? 50.882 : + index === 6 ? 54.3575 : 50; const range = new Range( thumbnailMean - THUMBNAIL_DOMAIN / 2, thumbnailMean + THUMBNAIL_DOMAIN / 2 ); ```

Here is launcher 2:

image
samreid commented 5 months ago

I also tried zooming in by setting the thumbnail domain to 10 and tested with autogenerate. It seemed reasonable. @matthew-blackman want to review/test/commit this?

matthew-blackman commented 5 months ago

It looks good to me, although the distribution of launcher 2 is heavily skewed left so we may want to use a value less than the mean. Let's review this with @catherinecarter to finalize these values.

catherinecarter commented 5 months ago

The centers still look a bit off to me. Here are all the launchers set to the same zoom level and bin width: Launchers 1, 2, 3 image image image Launchers 4, 5, 6 image image image

The dev.20 versions look more centered at the mean than the current version. Here's a video of switching between the dev.20 version and the version on main (you can tell which version is which because the dev.20 version has the default angle at 45 while the main version has the default at 30). For example, at about second 4, I am going back and forth between the versions for launcher 1. At about second 10, I flip between launcher 2's version. Roughly second number 17-18 going between launcher 3, seconds 22-23(ish) between launcher 4, 26-28 launcher 5, and 36-38 launcher 6.

https://github.com/phetsims/projectile-data-lab/assets/109630002/4d36e56d-92e7-46fa-a459-a5402d987615

Also, Launcher 6 looks like it has a better center, but the window isn't quite wide enough to fit all the data. The mean at 30 isn't in the thumbnail:

image

Do they all have the same x-axis scale? I thought the goal of the thumbnails was to 'zoom in' to the distributions according to their standard deviations, but I can't find an issue tracking our discussion. @matthew-blackman or @samreid, do you remember our conversation about the x-axis being a fixed number (e.g., 3.5 standard deviations from the mean for each launcher)? I can't remember what the conversation included or what we ended up deciding.

samreid commented 5 months ago

I wonder if there is a miscommunication since my proposal was in a patch, and it doesn't look like anyone committed the patch.

matthew-blackman commented 5 months ago

This is ready for review @catherinecarter. Here are the values we are using for the 6 mystery launchers. Feel free to recommend updates to values and I can re-post.

      const thumbnailDomain =
        index === 1 ? 20 :
        index === 2 ? 40 :
        index === 3 ? 30 :
        index === 4 ? 19 :
        index === 5 ? 20 :
        index === 6 ? 50;

      const thumbnailMean =
        index === 1 ? 46 :
        index === 2 ? 50 :
        index === 3 ? 55 :
        index === 4 ? 46 :
        index === 5 ? 51 :
        index === 6 ? 55;
catherinecarter commented 5 months ago

I played with it for a while. I found a couple instances on both launchers 2 and 6, sample size = 2, where there was a mean or two off the thumbnail. The graphs look so good, though, that I don't think it's worth updating for a few times when this will happen.

So I think the windows are looking great and don't need to be adjusted any more. Before closing this issue, however, I'd like to see what KF thinks to make sure. I'd also like his feedback on whether launcher 5 could have more zoom on the x-axis.

catherinecarter commented 5 months ago

I checked with KF, and he agrees that having random "rogue" means on the Sampling screen appear on the histogram but not the thumbnail is fine. He also agrees that the zoom level for launcher 5 is good. From Slack:

For a few means not appearing in the thumbnail:

I'd say that's fine for a few rogue means to be out of view in the thumbnails 🙂

For Launcher 5 x-axis zoom level:

I think I can see the variability reasonably well in the thumbnails, and I'm not sure zooming closer would make it better...

So this issue looks good to close.