phetsims / blackbody-spectrum

"Blackbody Spectrum" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
1 stars 3 forks source link

Color mapping seems incorrect for 1.0.2-rc.1 #109

Closed ariel-phet closed 5 years ago

ariel-phet commented 5 years ago

Comparing 1.0.2-rc.1 to the currently published version (and to the flash version), the color mapping seems incorrect.

In 1.0.2-rc.1 for the RGB display I am already seeing some color at 300 K.

Something is off here and the color mapping should probably closely mimic the flash version.

ariel-phet commented 5 years ago

Related to https://github.com/phetsims/QA/issues/376

ariel-phet commented 5 years ago

@KatieWoe to confirm for me and so she is aware of this current bug

KatieWoe commented 5 years ago

Good catch. This rc is slightly different than published. I will note that it is deliberately different from the flash version from what I understand (the visible spectrum used was changed slightly to conform with the range used by other html sims). @DianaTavares and @arnabp to determine what behavior is desired and where the change might have come from.

KatieWoe commented 5 years ago

The differences seems easiest to see at the extremes of the available temperatures. The first gif shows the highest temperature and that the intensity seems to be a bit different between the rc and the published version, but I can't see any color difference. The second gif shows the temperature I was able to start seeing color. slightintensityshift aboutequivalent It seems like it may be due to the change in increments, and it doesn't seem too worrying to me. But, @ariel-phet and @DianaTavares for final decision.

kathy-phet commented 5 years ago

Hi All, It seems like there are 2 points here:

1) The visible spectrum shown on the graph. Here the current HTML5 sim is more accurate in terms of its colors and where the color changes occur. The flash one is off a bit, having green where it should have already transitioned to yellowish-orange (my phd thesis was on 585nm light absorption so I know this color well).

2) The color shown on the composite image of the sun displayed. Here I think we need to match more what the flash sims shows in terms of when things appear "black" or like "nothing" ... right now it is showing a redish hue too early (at too low of a temperature). The flash sim is more accurate -- beginning to show the slighest dark red hue at 800K. Our version already shows a lot of red at 800K. Except if you actually look at the spectrum profile at the lower values you see no reason for a visible glow at those lower temperatures as the spectrum does not overlap with the visible. So this does need to change to match the flash version better.

Here is the page that talks about materials starting to glow at 800K. https://en.wikipedia.org/wiki/Incandescence

jonathanolson commented 5 years ago

It looks like one of the above commits WAS included on the 1.0 release branch for the simulation, but I don't see an RC or other evidence of testing.

This means that this change will be picked up in the RC made automatically for the automated maintenance for https://github.com/phetsims/phet-ios-app/issues/512, and we'd need to pay special attention otherwise it would get pushed out without being tested.

Has this been tested on the release branch? Should I make extra notes to QA to test this? Should I prevent this seem being production-deployed normally as part of the process?

arnabp commented 5 years ago

This is being tested in phetsims/QA#382. I talked with @jonathanolson and we will be coordinating two different RC tests before this sim is ready for production deployment.

ghost commented 5 years ago

Going off of https://github.com/phetsims/blackbody-spectrum/issues/109#issuecomment-514767304...

  1. It didn't sound like @kathy-phet wanted anything changed here.
  2. At 800K on macOS + Chrome + full brightness, I can just barely see a dark red sun, so đź‘Ť.
ghost commented 5 years ago

I may have spoken too soon. On my iPad Pro (iOS 12.3.1) + Safari + full brightness at 800K, I can't quite see the sun. @KatieWoe, it would be a good idea to check this on some of our test devices to see if the sun is visible at 800K.

kathy-phet commented 5 years ago

Hi Liam, at what temp can you see the start of a sun on that device? Thx

Sent from my iPhone

On Jul 27, 2019, at 12:12 PM, Liam notifications@github.com<mailto:notifications@github.com> wrote:

I may have spoken too soon. On my iPad Pro (iOS 12.3.1) + Safari + full brightness at 800K, I can't quite see the sun. @KatieWoehttps://github.com/KatieWoe, it would be a good idea to check this on some of our test devices to see if the sun is visible at 800K.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/phetsims/blackbody-spectrum/issues/109?email_source=notifications&email_token=ABG4KZB5CONFVF3XNUCDMBLQBSFZNA5CNFSM4IGSAYQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD26QEMA#issuecomment-515703344, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABG4KZBBQ6HPPT7SC6LU3BLQBSFZNANCNFSM4IGSAYQQ.

ghost commented 5 years ago

I can start to see the sun at 850K on that iPad and I see the same issue (not being able to see the sun at 800K) on Win10 + Firefox on my PC at home, but that may have to do with monitor settings etc.

kathy-phet commented 5 years ago

I would say this is all seems fine since it doesn’t take much higher to see it.

Sent from my iPhone

On Jul 27, 2019, at 2:57 PM, Liam notifications@github.com<mailto:notifications@github.com> wrote:

I can start to see the sun at 850K on that iPad and I see the same issue (not being able to see the sun at 800K) on Win10 + Firefox on my PC at home, but that may have to do with monitor settings etc.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/phetsims/blackbody-spectrum/issues/109?email_source=notifications&email_token=ABG4KZAKJATUA6UIA2S4BGDQBSZFBA5CNFSM4IGSAYQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD26SQ5Y#issuecomment-515713143, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABG4KZAOUG4WHQ3645NAFI3QBSZFBANCNFSM4IGSAYQQ.

KatieWoe commented 5 years ago

Confirming what @lmulhall-phet saw. I would agree with @kathy-phet, it looks pretty comparable to the flash version.