pr-omethe-us / PyKED

Python interface to the ChemKED database format
https://pr-omethe-us.github.io/PyKED/
BSD 3-Clause "New" or "Revised" License
14 stars 15 forks source link

Incorrect velocity units in RCM example #125

Open ischoegl opened 5 years ago

ischoegl commented 5 years ago

Code sample, preferably able to be copy-pasted and run with no changes

Cell [6] in rcm-example.ipynb, i.e.

from cansen.profiles import VolumeProfile
exp_time = dp.volume_history.time.magnitude
exp_volume = dp.volume_history.volume.magnitude
keywords = {'vproTime': exp_time, 'vproVol': exp_volume}
ct.Wall(reac, env, velocity=VolumeProfile(keywords));

uses wrong units for the velocity entry (should be m/s, not m^3/s normalized volume/time). This is incorrect, but nevertheless works as the wall area defaults to 1 m^2.

Expected behavior

The example should be compliant with Cantera's API . An easy fix would be to clarify the usage of normalized volume, or to separately specify piston area or calculate it from the cylinder bore (which unfortunately is not saved in the YAML file used as input, although several useful parameters were discussed in #91).

PyKED/ChemKED version, Python version, OS version

Current, all python/OS.

bryanwweber commented 5 years ago

I agree that this should be documented as using a normalized volume. It would probably be better to avoid the dependency on CanSen all together.

As for the cylinder bore, that is a fixed parameter that would be the same for every experiment run on a particular machine. We envision the ChemKED YAML files as only encoding data that will change on each experiment (T, p, phi, stroke, clearance, etc.). Hence, we think the bore belongs in a separate apparatus specification. Unfortunately, no such specification exists at the moment.

As I mentioned over at bryanwweber/CanSen#45, most RCM experiments that I'm aware of are not concerned with total quantities such as heat release, but only with intensive quantities that don't depend on the size of the cylinder, so the choice of using a normalized volume in the simulation is fine. That said, it should be possible to implement a VolumeProfile class that does not normalize the volume and requires the correct area be specified in the ct.Wall. I don't think that implementation should be included in the core PyKED code though.

ischoegl commented 5 years ago

Makes sense. As long as you specify the stroke and document volumes, the bore does not have to be specified, so the current ChemKED YAML format may be good as is.