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

Spectrometer design problems #48

Closed
pixelzoom opened this issue Aug 5, 2024 · 13 comments
Closed

Spectrometer design problems #48

pixelzoom opened this issue Aug 5, 2024 · 13 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Aug 5, 2024

Screenshots below show the Spectrometer in the Java and HTML5 versions. The Spectrometer has some significant design and usability problems, none of which have been addressed by the HTML re-design. Assigning to @DianaTavares and @arouinfar to kick-start the discussion.

Problems include:

  1. The y-axis does not scale. In the screenshot below for example, photons continue to be emitted for 97 nm, but there is no change shown on the Spectrometer.

  2. Some wavelengths overlap, making the spectrometer more difficult to read. For example, see 94 and 95 nm in the Java screenshot below. There's also overlap at 7460 nm, probably with 4053 nm, because the IR portion of the x-axis is compressed. There may be overlap between other wavelengths.

  3. One cannot tell what the emission wavelengths are by looking at the spectrometer. The x-axis is not labeled with the emission wavelengths. One must open the Absorption Wavelengths dialog to find the emission wavelength values -- not very intuitive or usable.

  4. There are 2 extra (unlabeled) x-axis tick marks between 380 nd 500 nm. By reading the code, I see that they are at 400 and 450, but this seems confusing.

  5. The Java screenshot below shows emissions near 780 nm in the IR spectrum. There is no emission wavelength near 780 nm, so this looks like a potential bug, see User question: incorrect wavelength in Bohr model #14.

Java:
screenshot_3450

HTML5 redesign:
screenshot_3451

@pixelzoom pixelzoom changed the title Design problems with Spectrometer Spectrometer design problems Aug 5, 2024
@DianaTavares
Copy link
Contributor

In today's design meeting, we proposed an option with lines:
image

But KP commented that the dots are important for the learning goal of comparing what transitions are more common. I will work on the mockup that addresses the problems described in this issue.

@DianaTavares
Copy link
Contributor

DianaTavares commented Aug 19, 2024

In this new version, I extended the space for 90-130 range to fit the dots:
image

But in today's design meeting with @arouinfar, @Nancy-Salpepi, @kathy-phet and @pixelzoom, we review that the pudding model emits a photon in the 150 nm, and that value is not in the range.

@pixelzoom said that he will try to fit all the range. But we agree in:

  • Place arrows at the top when you have revised the limit of points that can fit on the graph, indicating that the number is larger.

  • add a small number in the top (same color that the dot) indicating the value of the wavelength (on the mockup, I don't put all the values, but in the sim should)

  • Delete the record button.

  • The graph can be divided into ranges (like the mockup, that include 3 different ranges for UV, visible and IR) to fit the values, and 150 need to be included.

@pixelzoom
Copy link
Contributor Author

We also concluded in today's design meeting that the numbers at the top of each stack of photons do not need to be present in the snapshots. The numbers would be unreadable, and the snapshots are intended for qualitative comparison.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Dec 30, 2024

I've made progress on the spectrometer and snapshots. But I'm going to pause, because there are many design-related problems. So this will need a design meeting.

  • The first problem is that we do not have enough horizontal space. In the first screenshot below, I've broken the x-axis up into 3 sections (UV, visible, IR) each with a different length and range that has been tweaked to prevent overlap of x-axis labels (wavelength) and data. As you can see, the Spectrometer is way outside the right edge of the safe bounds (red rectangle.)

  • As I got into implementing, putting the wavelength values at the top of the bars seemed like a bad decision. Labels typically go on the axis that they are associate with. In a bar graph, y-axis values are sometimes at the top of each bar. Our decision to put x-axis labels (wavelengths) at the top of each bar makes these values look like the y-axis value (number of protons). So I did not do what we had decided. Instead, I put labeled tick marks on the x-axis. Ticks appear when there is data.

  • There is no room for axes labels. How will the user know that x-axis in wavelength in nm, y-axis is number of photons emitted.

  • There is no room to label the min/max for the 3 x-axis sections. Doing so results in overlap with emission wavelengths.

  • There is room for "UV" and "IR" labels. But their positions would need to be hardcoded, and they will not look centered, so will look odd. We should probably address this after figuring out how to make the spectrometer fit into the horizontal space available.

  • We has previously decided not to include x-axis values (wavelengths) in the snapshots, and that's what is shown in the second screenshot below. Is that still OK?

  • Discuss layout of the snapshots dialog. Does it need a title? Are the snapshot labels OK? Is the location of the trash buttons OK? (I can't do anything about the whitespace below the close button, that's baked into sun.Dialog.)

  • We previously decided to get rid of the Record/Pause button. So the spectrometer is (currently) always recording when the accordion box is expanded, and not recording when the accordion box is collapsed. And it is cleared when pressing the erase button, and when switching hydrogen atom models. Is this the behavior that we want?

  • Snapshots (model and view) are currently implemented as dynamic elements. This is not going to work with PhET-iO. So before I rework the code... How do we want snapshots to present in Studio and the PhET-iO API?

A couple of query parameters to know about if you'd like to test drive:

  • debugSpectrometer causes the spectrometer to show maximum bars for all possible emission wavelengths, as in the screenshots below.
  • debugSpectrometerData displays spectrometer data in text-only format, in the upper-right corner of the Spectrometer and snapshots.

spectrometer

snapshots

@DianaTavares
Copy link
Contributor

I don't have the answer to all the comments here, but I want to put my ideas here to be disused in the design meeting:

The first problem is that we do not have enough horizontal space. In the first screenshot below, I've broken the x-axis up into 3 sections (UV, visible, IR) each with a different length and range that has been tweaked to prevent overlap of x-axis labels (wavelength) and data. As you can see, the Spectrometer is way outside the right edge of the safe bounds (red rectangle.)

Now, the Spectrometer includes all the UV range:
image
The proposal is that UV goes only until 130 nm, because the rest of the values are not used. The bigger wavelength of the photon emitted is 122. We can use diagonal lines to let the user know that there is a "jump" of values:
image

As I got into implementing, putting the wavelength values at the top of the bars seemed like a bad decision. Labels typically go on the axis that they are associate with. In a bar graph, y-axis values are sometimes at the top of each bar. Our decision to put x-axis labels (wavelengths) at the top of each bar makes these values look like the y-axis value (number of protons). So I did not do what we had decided. Instead, I put labeled tick marks on the x-axis. Ticks appear when there is data.

Agree with this!

There is no room for axes labels. How will the user know that x-axis in wavelength in nm, y-axis is number of photons emitted

Java use to have it in the title of panel:
image
We can display it only when the spectrometer is open.

There is no room to label the min/max for the 3 x-axis sections. Doing so results in overlap with emission wavelengths.
There is room for "UV" and "IR" labels. But their positions would need to be hardcoded, and they will not look centered, so will look odd. We should probably address this after figuring out how to make the spectrometer fit into the horizontal space available.

This is my idea:
image

We has previously decided not to include x-axis values (wavelengths) in the snapshots, and that's what is shown in the second screenshot below. Is that still OK?

Now that I see them in the sim without the values, looks like we lost something important. I think is better to add the values. My idea to make them fit is reducing the dots vertically that can be added (but the number of max dots in the snapshot need to be the same that in the Spectrometer. Now in the sim are 14, and they can be 10), to allow the values in the bottom.

image

Discuss layout of the snapshots dialog. Does it need a title? Are the snapshot labels OK? Is the location of the trash buttons OK? (I can't do anything about the whitespace below the close button, that's baked into sun.Dialog.)

Does it need a title?: I Think it is ok without a title.
Are the snapshot labels OK? Yes.
Is the location of the trash buttons OK? I like them inside the data, as it is in the picture above, but if that is not possible, aside is ok!

We previously decided to get rid of the Record/Pause button. So the spectrometer is (currently) always recording when the accordion box is expanded, and not recording when the accordion box is collapsed. And it is cleared when pressing the erase button, and when switching hydrogen atom models. Is this the behavior that we want?

Yes, but that is something to test in the interviews. One question, does the data collection stop if I first have the panel open, start to collect data, and then I close the spectrometer?

Snapshots (model and view) are currently implemented as dynamic elements. This is not going to work with PhET-iO. So before I rework the code... How do we want snapshots to present in Studio and the PhET-iO API?
No ideas here!

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jan 2, 2025

@DianaTavares thanks for adding your comments. Looking forward to discussing in design meeting.

Re:

The proposal is that UV goes only until 130 nm, because the rest of the values are not used. The bigger wavelength of the photon emitted is 122. We can use diagonal lines to let the user know that there is a "jump" of values:

The UV range has an upper value of 150 nm, which is the wavelength emitted by the Plum Pudding model. Again, if you run with ?debugSpectrometer you will see a bar at all possible emission wavelengths. So the current implementation has made each range (UV, visible, IR) as short as possible.

@DianaTavares
Copy link
Contributor

The UV range has an upper value of 150 nm, which is the wavelength emitted by the Plum Pudding model. Again, if you run with ?debugSpectrometer you will see a bar at all possible emission wavelengths. So the current implementation has made each range (UV, visible, IR) as short as possible.

Well, one idea now is to change the ranges of the max UV only for Plum Pudding. No to change the size of the axes, just in Plum Pudding, that the max is 160 for example, and in the others are 130:
image

Also, other idea is not to use a lineal scale for UV and IR

@DianaTavares
Copy link
Contributor

DianaTavares commented Jan 2, 2025

Design Meeting January, 2nd
CM, DL, NS, AP, KP, KW

scale to fit

  • The spectrometer will be compressed and the values remapped for UV and IR.
  • Also, shorten visible spectrum on the red end
  • CM going to hard code in UV and IR labels
  • add // in the whitespace between the segments (and try usual breaks on the ends of axis segments)

Snapshoots
Leave the 4 snapshots and not add data.
Trash button is ok the place it is.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jan 3, 2025

@DianaTavares This is ready for review.

First, I should note that I shortened the monochromatic wavelength slider from 250 to 200 in order to gain some horizontal space for the Spectrometer. (I believe @Nancy-Salpepi suggested that in design meeting.) I don't see any problems, but keep that in mind as you review.

Next, here's a summary of the changes:

The spectrometer x-axis has standard '/ /' breaks between the 3 sections. The scale of the 3 sections is compressed (remapped) and it's easy to adjust the mapping for any of the wavelengths. The visible spectrum has been shortened at the right (red) end. I reduced the maximum number of photons in each "bar" from 14 to 12, and was able to increase the font size for tick labels. Here's a screenshot running with ?debugSpectrometer, which shows maximum bars for all possible wavelengths:

screenshot_3683

Showing x-axis labels and tick mark labels is a problem, there is no space for this. So as a compromise, axis labels are shown until there is data. As soon as there is data, tick labels appear, and the x-axis labels disappear. Hopefully seeing the x-axis labels when the spectrometer is empty will be informative enough, and not confusing when the labels disappear. Here's a screenshot:

screenshot_3686

We have no room to add units for the axes, so (as you suggested, and as in the Java version) I added them to the accordion box title when expanded:

screenshot_3684

When the accordion box is collapsed, the units are not shown:

screenshot_3685

... but I'm not sure if we should bother changing the title when collapsed. It adds code complexity, it adds an extra string to translate. And perhaps most importantly, I'm not sure how description will deal with a dynamic title on AccordionBox. What do you think?

Snapshots use the same scale as the Spectrometer, and we are able to show tick labels. For example:

screenshot_3687

Finally... Unlike the "Electron Energy Levels" accordion box, I think we should keep the border around the spectrometer display area. Having a well-defined titlebar area seems important because we have push buttons in the titlebar. Your opinion?

screenshot_3688

@DianaTavares
Copy link
Contributor

I love the implementation! It looks amazing.

but I'm not sure if we should bother changing the title when collapsed. It adds code complexity, it adds an extra string to translate. And perhaps most importantly, I'm not sure how description will deal with a dynamic title on AccordionBox. What do you think?

For me, it is ok to leave the title as "Spectpmeter (photons emitted / nm)" when the panel is collapsed. I agree it is unnecessary to add complexity to other stuff. Leave it the same!

Finally... Unlike the "Electron Energy Levels" accordion box, I think we should keep the border around the spectrometer display area. Having a well-defined titlebar area seems important because we have push buttons in the titlebar. Your opinion?

when both panels are collapsed, looks strange that they are the same control, but they look different,
image

Well, I have the same thinking when they are open:
image

I understand that Spectrometer has the buttons. In that case, for me is better that both panels look the same, my suggestion is that the electron Energy Panel also includes a gray background around.

@pixelzoom pixelzoom assigned pixelzoom and unassigned DianaTavares Jan 3, 2025
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jan 3, 2025

Thank for the review @DianaTavares. I will make the following changes:

  • Keep the same (longer) title on the Spectrometer accordion box, regardless of whether it's expanded or collapsed.
  • Make both accordion boxes have the same look that the Spectrometer accordion box currently has.

@pixelzoom
Copy link
Contributor Author

@DianaTavares This is ready for review, close if OK.

I ended up making the Spectrometer accordion box look like the Electron Energy Levels accordion box. I though I was not going to like the buttons floating in the upper-right corner, but I think it looks OK.

screenshot_3690

@DianaTavares
Copy link
Contributor

It looks amazing, thanks!!

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

3 participants