Jrosen48

[jrosen48]

Added a modified Diffusion & Temperature simulation, 2-temperature-mvs-ap-physics-2, for potential use in AP Physics 2 class associated with Michigan Virtual Schools.

[ddamelin]

Josh, I'm very impressed with how far you got with this without a programming background. There are some things a programmer would do differently, but the code you have is quite workable. My suggestions fall into two categories, those I'd like you to respond to before we merge this pull request, and suggestions in case you want to improve things at the code level.

Please respond to the following:

• There is a directory for interactives that we host for other folks. You should move 2-temperature-mvs-ap-physics-2.json to a new directory that you can make (perhaps call it "MSU") or use the "CREATE" directory that is already there in src/interactives/external-projects
• The interactive you created refers to the same model as the original interactive that you built off of. It would be safest to make a duplicate of that model and refer to that. The one currently being referred to is: models-converted/lab-version/1/md2d/sam-activities/diffusion/DiffusionTemp/DiffusionBarrier$0.json I would put the duplicate in models/lab-version/1/md2d/external-projects/CREATE (or if you make a new directory for the interactive, then make a new directory here with the same name. (Also, rename it without using the $ in the name.)
• Remove the i18nMetadata reference from your interactive. This refers to a location for translations of that interactive. Unless you want to translate this to other languages, this should be removed.

Suggestions for improving the coding:

• You should be able to move the set pressure call on line 31 before the onPropertyChange function. As it is now it calls this set pressure function every tick of the model. You already have it change when the temperature is changed, so this is not necessary here. In fact you could remove it completely and just use the initialValue property of the pressure parameter you created to set this. If you are determined for it to read 0 when the model is not running there is an onStart() function that you can call which would be more appropriate.
• You currently calculate the total kinetic energy, but this is a property of the model that you can get with get('kineticEnergy').
• You could set the min and max of the temperature slide to be the actual temperature values and lock the slider to the values in your array by using the "steps" property of the slider component. Then you could assume the value in the onChange property of the selectedTemperature parameter will be the actual temperature, so there is no need for the array.
• At first the value of 1.37 in your pressure calculation was a bit baffling to me until I realized you manually calculated the volume of the model and hardcoded the answer here. To make this more readable you might have created a volume property and used model properties of "width" and "height" to define its value, making the meaning more apparent.
• I see that you are calculating the theoretical pressure here. You may want to make sure the units are correct given the kinetic energy values are in eV. Do you need some conversion factor here. Not sure, but you should double-check.

[jrosen48]

Hi Dan, and thank you for such helpful comments and feedback. Working on this has been a good learning experience and I'm very grateful for help. I made changes in response to needed changes and suggestions for improving coding - in terms of needed changes, I made a directory in "external-projects", "msu" for the interactive and model (I made a duplicate of this with a different name as suggested) and removed the metadata reference.

Regarding suggestions for improving the coding, I:

• Determined using the pressure probe to calculate pressure, instead of calculating the theoretical pressure, adapted based on how it was done in the Pressure Equilibrium interactive.
• Used get('kineticEnergy') instead of calculating the total kinetic energy.
• Set the min and max of the temperature slide to the actual temperature.
• Created parameters for the model height and width, and then calculated volume (as an output) - is it perhaps better to calculate volume as a parameter?

I think adding the Pressure Gauge might made the components at the bottom get scrunched up and so one question I have is whether the simulation can be displayed a bit wider.

Thank you again for the help!

[ddamelin]

I'll take a look today.

On Aug 18, 2016, at 3:48 PM, Joshua Rosenberg notifications@github.com wrote:

Hi Dan, and thank you for such helpful comments and feedback. Working on this has been a good learning experience and I'm very grateful for help. I made changes in response to needed changes and suggestions for improving coding - in terms of needed changes, I made a directory in "external-projects", "msu" for the interactive and model (I made a duplicate of this with a different name as suggested) and removed the metadata reference.

Regarding suggestions for improving the coding, I:

Determined using the pressure probe to calculate pressure, instead of calculating the theoretical pressure, adapted based on how it was done in the Pressure Equilibrium interactive. Used get('kineticEnergy') instead of calculating the total kinetic energy. Set the min and max of the temperature slide to the actual temperature. Created parameters for the model height and width, and then calculated volume (as an output) - is it perhaps better to calculate volume as a parameter? Thank you again for the help!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[ddamelin]

Not sure you have completely pushed all changes. This version is slightly broken. Here are some further suggestions:

• The "pressure-output" component is mentioned in the layout but no longer exists.
• Your current pressure measure doesn't seem to work. As written it says to use the obstacle that separates the two gasses as a pressure sensor, but then this obstacle is removed. If you wanted to measure pressure this way, then you would need to add some permanent obstacle to the model. Depending on your purposes, it might be better to go back to calculating the theoretical pressure based on number of atoms and temperature. If you use the obstacle as a pressure sensor the values will go up and down as it averages out the force of each atomic collision. You won't get a steady number like you do with the calculated pressure. But you may want this or you may not. It is up to you. If you are going to add an obstacle to the model I would put a very thin one on the right side of the model, one that is not visible. This would probably be a bit tricky for you, so feel free to ask for help in doing that, or just revert to calculating the theoretical pressure.
• You could eliminate the totalElementKineticEnergy output and just use kineticEnergy for your numerical output. However, your current approach causes the KE to change to zero if they stop the model. This is a bit strange because the model is just frozen in time, it hasn't really changed in such a way as to have atoms that really have no kinetic energy. In fact it might be considered contradictory to say the model is at some temperature but has no total KE, which is what the model does now. So, I would simplify this by getting rid of the custom totalElementKineticEnergy and just use kineticEnergy in the numerical output.
• You can simplify the volume calculation further by just creating a parameter called "volume" and using get('width') and get('height'). You don't need separate parameters to set these values manually. They are properties of the model.
• To have the new grouping you made under external-projects show up in the pull-down menu you need to edit the groups.yaml file to add the msu group. See the end of the file to add a new entry specifying the external-projects/msu path.

[jrosen48]

Thank you, I think something was a bit funky, I'm working on changes in response to these suggestions. Thank you again!

On Fri, Aug 19, 2016, 2:20 PM ddamelin notifications@github.com wrote:

Not sure you have completely pushed all changes. This version is slightly broken. Here are some further suggestions:

• The "pressure-output" component is mentioned in the layout but no longer exists.
• Your current pressure measure doesn't seem to work. As written it says to use the obstacle that separates the two gasses as a pressure sensor, but then this obstacle is removed. If you wanted to measure pressure this way, then you would need to add some permanent obstacle to the model. Depending on your purposes, it might be better to go back to calculating the theoretical pressure based on number of atoms and temperature. If you use the obstacle as a pressure sensor the values will go up and down as it averages out the force of each atomic collision. You won't get a steady number like you do with the calculated pressure. But you may want this or you may not. It is up to you. If you are going to add an obstacle to the model I would put a very thin one on the right side of the model, one that is not visible. This would probably be a bit tricky for you, so feel free to ask for help in doing that, or just revert to calculating the theoretical pressure.
• You could eliminate the totalElementKineticEnergy output and just use kineticEnergy for your numerical output. However, your current approach causes the KE to change to zero if they stop the model. This is a bit strange because the model is just frozen in time, it hasn't really changed in such a way as to have atoms that really have no kinetic energy. In fact it might be considered contradictory to say the model is at some temperature but has no total KE, which is what the model does now. So, I would simplify this by getting rid of the custom totalElementKineticEnergy and just use kineticEnergy in the numerical output.
• You can simplify the volume calculation further by just creating a parameter called "volume" and using get('width') and get('height'). You don't need separate parameters to set these values manually. They are properties of the model.
• To have the new grouping you made under external-projects show up in the pull-down menu you need to edit the groups.yaml file to add the msu group. See the end of the file to add a new entry specifying the external-projects/msu path.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/concord-consortium/lab-interactives-site/pull/58#issuecomment-241095301, or mute the thread https://github.com/notifications/unsubscribe-auth/AEYh9r5oXt3fhg-5iDwfMGEXoEW0UPdjks5qhfPngaJpZM4JieUG .

[jrosen48]

Hi Dan, to address these suggestions, I:

• went back to calculating theoretical pressure (am not 100% certain the units are correct)
• used kineticEnergy as the numeric output for Total KE
• simplified the volume calculation
• added a new grouping for MSU interactives

I also slightly changed the layout, so the parameters and output are all on the bottom. Thanks again for all of your help!

[ddamelin]

I'm currently on vacation, but will try to take a look this weekend.

[jrosen48]

Thank you, much appreciated (but no rush on my end).

On Fri, Aug 26, 2016, 12:05 AM ddamelin notifications@github.com wrote:

I'm currently on vacation, but will try to take a look this weekend.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/concord-consortium/lab-interactives-site/pull/58#issuecomment-242625173, or mute the thread https://github.com/notifications/unsubscribe-auth/AEYh9qjSG7FZubOWtFJO8esB66nbWdr-ks5qjmXkgaJpZM4JieUG .

[ddamelin]

This looks almost ready. There are a few more things.

• You might want to consider using text for the play button by including: "controlButtonStyle": "text", just below the "controlButtons" property.
• The units for kinetic energy are eV. If you need them displayed in J, then you'll need to do some conversion.
• The pressure calculation and units still seem not quite right to me. A simpler way to calculate it would be using the ideal gas law pV = nRT. You'll need to find a version of R (or convert one yourself) that has the right units. So one could take R = 0.0821 L•atm/mol•K and convert to nm^3•atm/mol•K. Then you would get pressure in atm (or continue converting to whatever pressure unit you want). n = moles of atoms, T = temp in Kelvin

[jrosen48]

Hi again Dan, thanks, I addressed these pieces of feedback by:

• adding the text for the play button.
• trying to change the units for kinetic energy to eV, but running into some trouble - I don't think I had set the units to J (that seemed to be the default unit abbreviation), so I tried to add "units: "eV"" as a property for the "Total KE" numericOutput, and this removed the "J" appearing next to the values, but the "eV" didn't seem to appear. I may be missing something obvious here, and hope there's not an obvious oversight on my part!
• brushing up on unit conversions and calculaing pressure as you suggested, however, the number is very tiny--not surprising given the number of atoms, but so tiny that I'm concerned I may have calculated the pressure incorrectly here.
  • for moles of atoms, I simply used getNumberofAtoms(), and wonder if that or the conversion factor between L and nm^3 is a problem. Or, whether I simply need to convert the units to be something more appropriate for the pressure as calculated.

Thank you again. Looking forward to finalizing this and next steps.

[ddamelin]

There are two issues remaining.

1. I didn't realize before, but this model has the model option "unitsScheme" : "mks" which is normally used for models that are trying to mimic some macro level phenomena. What we do is basically scale up the model as if these are not atoms, but physical balls. So, the distances, masses and velocities are all scaled up. The simplest way to solve your problem would be to remove the Total KE output. Is this crucial to your use? If it is then we can either take the model out of "mks" mode, which will have it run in femtoseconds or we can try to create a property that re-converts the energy (which is reported in Joules) back down to what it would be if these were atoms.
2. The pressure calculation is still not correct. The "n" in PV = nRT is number of moles of atoms, not the number of atoms.

So my suggestion for the quickest way forward would be to remove the Total KE output, and to change the pressure calculation to: return (getNumberOfAtoms()/6.02e+23) * ((.082) * 1e+24) * get('targetTemperature')) / get('volume')

I checked the atomic radius of the atoms used in this simulation and it is 0.14 nm, so you might want to adjust the thickness of your container to be 0.14 nm rather than the current 0.1

You will end up seeing a pretty high pressure in atm, but I think that is reasonable given how tightly packed these molecules are at high temperatures.

[jrosen48]

Hi Dan,

I made changes to address the issues:

• Removed Total KE
• Recalculated pressure

Thanks again. My biology is showing.

Josh

[ddamelin]

Looks good. I'm pushing the button on this.