phetsims / circuit-construction-kit-common

"Circuit Construction Kit: Basics" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
10 stars 10 forks source link

capacitanceStep is smaller than capacitorNumberDecimalPlaces #862

Closed oliver-phet closed 1 year ago

oliver-phet commented 2 years ago

When setting the value for the capacitance C of a capacitor, the display on shows two decimal places (like 0.08 F). However, clicking on the slider-arrows modifies the capacitance further (without showing the fine-tuning in the display), as evidenced by a voltmeter reading for the capacitor terminals.

It would be good to see more decimal places in the capacitance since the slider/arrow-buttons allows this fine-tuning!

I was trying to set up a virtual demo with two capacitors 0.08F and 0.16F (so that C2= 2*C1) connected to a 9-v battery.

From the visuals below, the displayed values suggest that V1=5.88 V2=3.12 so that I have achieved V1/V2=1.88 (not 2). I would have to tweak the capacitance sliders [in an alternating manner?] while looking at voltmeters to fine-tune to the desired capacitances.

I would rather see more decimal places in the capacitance. It would also be nice to allow me to type-in precisely my desired capacitances (within the bounds) rather than tune to it with the slider.

Without these additional decimal places displayed, I think it would be difficult to ask to student to setup a situation as above (with C2=2*C1) and verify the algebraic results.

8652F11CFE7A4A47B77F992844708E99 EB1241EDB35D4D2E884ACE3DBBFA1ECE

arouinfar commented 2 years ago

@oliver-phet this looks like an error on our part. It's going to take some digging to find the paper trail on what the desired behavior should be, but in the meantime please ask the user to use ?capacitorNumberDecimalPlaces=3 to correct things in the published version. Alternatively, they can use ?capacitanceStep=0.01 to reduce the increment of the tweaker buttons to match the number of decimals provided in the display. These were not intended to be a public-facing query parameters, just something we used in our own testing/model tuning but it works in the published version.

arouinfar commented 2 years ago

The capacitance range was determined in https://github.com/phetsims/circuit-construction-kit-common/issues/590, but the query parameters were introduced in https://github.com/phetsims/circuit-construction-kit-common/issues/780#issuecomment-974291874 when we were trying to address a situation with buggy inductors. We ended up keeping the capacitance values as they are and limiting the number of inductors instead. However, we kept the query parameters around, including the incongruent default values of capacitanceStep=0.001 and capacitorNumberDecimalPlaces=2.

This is something we should be able to easily fix in a maintenance release, but there are two ways we could go:

  1. Increase the default value of capacitorNumberDecimalPlaces to 3.
  2. Change the default value of capacitanceStep to 0.01.

Based on the decisions made in #590 we probably should go with option (2). However, there may have been some other reason for changing capacitanceStep to 0.001 that I'm just not remembering. @ariel-phet @samreid do you recall?

ariel-phet commented 2 years ago

@arouinfar I agree option (2) sounds like the right solution. I believe that was our intention. We have fine enough control with inductor values to tune in a resonant frequency.

arouinfar commented 2 years ago

Thanks @ariel-phet. Unless @samreid has objections, we should proceed with updating the capacitanceStep default to 0.01 and publishing a MR of CCK: AC and CCK: AC - VL.

arouinfar commented 2 years ago

2/24/22 design meeting. We decided to proceed with option (2) and publish MR of CCK: AC and CCK: AC - VL. @samreid should also confirm that the slider thumb snaps to 0.01 increments. In Studio, using capacitanceStep=0.01 seems to behave as desired.

samreid commented 2 years ago

I changed the capacitanceStep to 0.01 and made it the slider thumb snap step as well. @arouinfar can you please verify the behavior on phettest before we create the maintenance releases? After the behavior is verified do we need any other steps such as QA involvement for this maintenance release?

arouinfar commented 2 years ago

@samreid looks good in master. The QA queue looks pretty quiet, so we can probably ask them to sanity check once you patch the release branch. This is a very minor change, so I am also fine with self-testing if you'd rather go that route.

samreid commented 2 years ago

@arouinfar can this be included with the next milestone of CCK (PhET-iO)? Or does it warrant an earlier separate maintenance release?

arouinfar commented 1 year ago

@samreid the PhET-iO milestones are all related to CCK: DC, and it will likely be quite some time before CCK: AC & CCK: AC - VL are republished off of master. Let's proceed with publishing maintenance releases of the AC flavors, as time allows.

samreid commented 1 year ago

On Thursday, @jonathanolson reported that he is working on maintenance releases. He said:

I'll be working on maintenance release things. Please don't do anything on maintenance branches without talking to me first!

I replied:

@jonathanolson Can I work on https://github.com/phetsims/circuit-construction-kit-common/issues/862 ?

On hold until we know more.

jonathanolson commented 1 year ago

Maintenance complete that was blocking, feel free to move ahead!

samreid commented 1 year ago

Today's meeting: @arouinfar @matthew-blackman and me: If we are publishing AC within a couple of months, that's OK. But if it would be longer than that, we should cherry-pick that commit.

samreid commented 1 year ago

I feel it could be a couple of months before AC is published. So I'll add this to our team's backlog.

samreid commented 1 year ago

Oops, I recall @jonathanolson said another maintenance release is in progress. On hold until we get the all-clear.

samreid commented 1 year ago

@jonathanolson reported the other maintenance release is complete and we can proceed.

samreid commented 1 year ago

Build succeeded: Sim = circuit-construction-kit-ac Version = 1.0.11-rc.1 Brands = phet Locales = en

samreid commented 1 year ago

RCs are here: https://phet-dev.colorado.edu/html/circuit-construction-kit-ac/1.0.11-rc.1/phet/ https://phet-dev.colorado.edu/html/circuit-construction-kit-ac-virtual-lab/1.0.10-rc.2/phet/

@arouinfar can you please spot check them?

arouinfar commented 1 year ago

Thanks @samreid. I spot-checked the rc's and everything looks good.

samreid commented 1 year ago

I sent the build production commands over 10 minutes ago. Not sure why the build server takes so long.

samreid commented 1 year ago

Publications complete. @oliver-phet do we need to notify someone that wrote to phethelp?

oliver-phet commented 1 year ago

User emailed via phethelp, though the 3 decimal place query parameter is likely still most useful to them.