-
Notifications
You must be signed in to change notification settings - Fork 16
Added new chemked schema for new experiment types #132
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
f794cb7
d3c3807
3b44a86
c204570
c8b2542
b6e4383
cdd775a
0239951
d327f44
efae6c2
5080454
711d152
fdfd7a9
55f8f2a
45ff61f
81f06af
fc40b8c
27c2b54
0ff6e79
6f5b483
8fc4ac3
09abd79
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,7 +19,9 @@ | |||||||
|
|
||||||||
| # Valid properties for ReSpecTh dataGroup | ||||||||
| datagroup_properties = ['temperature', 'pressure', 'ignition delay', | ||||||||
| 'pressure rise', | ||||||||
| 'pressure rise', 'laminar burning velocity', | ||||||||
| 'distance', 'flow rate', 'residence time', | ||||||||
| 'volumetric flow in reference state', | ||||||||
|
||||||||
| 'volumetric flow in reference state', | |
| 'volumetric flow in reference state', | |
| 'volumetric flow rate in reference state', |
Copilot
AI
Mar 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exp_type_text is taken directly from root.find('experimentType').text without stripping whitespace. If the XML contains leading/trailing whitespace/newlines (common in pretty-printed files), the lookup and the case-insensitive fallback will fail and raise NotImplementedError for an otherwise supported experiment type. Using root.findtext('experimentType', '').strip() (and raising MissingElementError('experimentType') when empty) would make this parsing robust and align error handling with other required elements like apparatus/kind.
Copilot
AI
Mar 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_experiment_kind() now accepts any apparatus/kind (and apparatus/mode) text and passes it through unchanged. Since the ChemKED schema constrains apparatus.kind/mode to an allowed set, this can cause ReSpecTh_to_ChemKED(..., validate=True) or later ChemKED validation to fail with an unhelpful error for unexpected but mappable ReSpecTh values. Consider normalizing/mapping known ReSpecTh variants to the schema’s allowed strings and raising a targeted NotImplementedError (or KeywordError) when the kind/mode can’t be mapped.
Copilot
AI
Mar 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_experiment_kind() behavior changes (new supported experiment types, relaxed apparatus kind validation, optional apparatus.mode) will require updating pyked/tests/test_converters.py expectations and adding coverage for at least one non-ignition experiment type + apparatus/mode parsing. Without corresponding test updates, the existing converter test suite will fail or no longer verify the intended behavior for these new branches.
Copilot
AI
Mar 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ReSpecTh_to_ChemKED() now allows non-ignition experiment types via get_experiment_kind(), but it still unconditionally calls get_datapoints(), whose docstring/logic is ignition-delay-specific (e.g., it only supports the ignition-delay dataGroup structure and will reject other required columns like time for concentration time profiles). This means supported experiment types can fail later with unrelated KeyError/KeywordError instead of a clear “not implemented” message, and it doesn’t produce datapoints matching the new non-IDT schemas. Either keep rejecting non-ignition types in this converter until per-type datapoint parsing is implemented, or add experiment-type-specific datapoint parsing that matches the new schemas.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| # Schema for burner stabilized flame speciation measurement datapoints | ||
| burner-stabilized-flame-speciation-measurement-schema: &burner-stabilized-flame-speciation-measurement-schema | ||
| type: list | ||
| minlength: 1 | ||
| schema: | ||
| type: dict | ||
| schema: | ||
| pressure: *value-unit-required | ||
| temperature: *value-unit-required | ||
| composition: *composition | ||
| equivalence-ratio: | ||
| type: float | ||
| min: 0.0 | ||
| distance: *value-unit-required | ||
| flow-rate: *value-unit-optional | ||
| measured-composition: *composition |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,6 +7,11 @@ | |||||||||
| !include value_unit_schema.yaml | ||||||||||
| !include composition_schema.yaml | ||||||||||
| !include ignition_delay_schema.yaml | ||||||||||
| !include laminar_burning_velocity_measurement_schema.yaml | ||||||||||
| !include concentration_time_profile_measurement_schema.yaml | ||||||||||
| !include jet_stirred_reactor_measurement_schema.yaml | ||||||||||
| !include outlet_concentration_measurement_schema.yaml | ||||||||||
| !include burner_stabilized_flame_speciation_measurement_schema.yaml | ||||||||||
| ###################################################### | ||||||||||
|
|
||||||||||
| # Common reference for authors' information | ||||||||||
|
|
@@ -26,9 +31,16 @@ common-properties: | |||||||||
| type: dict | ||||||||||
| schema: | ||||||||||
| pressure: *value-unit-optional | ||||||||||
| temperature: *value-unit-optional | ||||||||||
| ignition-type: *ignition-type | ||||||||||
|
||||||||||
| ignition-type: *ignition-type | |
| ignition-type: | |
| <<: *ignition-type | |
| required: false |
Copilot
AI
Apr 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
apparatus.kind allowed values include "stirred reaction", which looks like a typo (likely meant "stirred reactor"). Keeping the misspelling in the enum will allow invalid/undesired values and make downstream normalization harder.
| - stirred reaction |
Copilot
AI
Mar 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The apparatus.mode.allowed list contains entries with : (e.g., extrapolation method to zero stretch : LS). In YAML, an unquoted : followed by a space is parsed as a mapping separator, so these list items will not be treated as plain strings and can break schema loading/validation. Quote these values (or remove the colon/space pattern) so they remain string literals in the allowed list.
| - extrapolation method to zero stretch : LS | |
| - extrapolation method to zero stretch : NQ | |
| - "extrapolation method to zero stretch : LS" | |
| - "extrapolation method to zero stretch : NQ" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| # Schema for concentration time profile measurement datapoints | ||
| # | ||
| # time-shift defines the t=0 reference for the profile | ||
| time-shift: &time-shift | ||
| type: dict | ||
| schema: | ||
| target: | ||
| required: true | ||
| type: string | ||
| type: | ||
| required: true | ||
| type: string | ||
| allowed: | ||
| - half decrease | ||
| - relative decrease | ||
| amount: *value-unit-optional | ||
|
|
||
| concentration-time-profile-measurement-schema: &concentration-time-profile-measurement-schema | ||
| type: list | ||
| minlength: 1 | ||
| schema: | ||
| type: dict | ||
| schema: | ||
| pressure: *value-unit-required | ||
| temperature: *value-unit-required | ||
| composition: *composition | ||
| equivalence-ratio: | ||
| type: float | ||
| min: 0.0 | ||
| concentration-profiles: | ||
| type: list | ||
| required: true | ||
| minlength: 1 | ||
| schema: | ||
| type: dict | ||
| schema: | ||
| species-name: | ||
| type: string | ||
| required: true | ||
| InChI: | ||
| type: string | ||
| SMILES: | ||
| type: string | ||
| quantity: | ||
| required: true | ||
| type: dict | ||
| schema: | ||
| units: | ||
| required: true | ||
| type: string | ||
| time: | ||
| required: true | ||
| type: dict | ||
| schema: | ||
| units: | ||
| required: true | ||
| type: string | ||
| values: | ||
| required: true | ||
| type: list | ||
| minlength: 2 | ||
| schema: | ||
| type: list | ||
| oneof_items: | ||
| - - type: float | ||
| - type: float | ||
| - - type: float | ||
| - type: float | ||
| - type: float | ||
| time-shift: *time-shift |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| # Schema for jet stirred reactor measurement datapoints | ||
| jet-stirred-reactor-measurement-schema: &jet-stirred-reactor-measurement-schema | ||
| type: list | ||
| minlength: 1 | ||
| schema: | ||
| type: dict | ||
| schema: | ||
| pressure: *value-unit-required | ||
| temperature: *value-unit-required | ||
| composition: *composition | ||
| equivalence-ratio: | ||
| type: float | ||
| min: 0.0 | ||
| measured-composition: *composition |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| # Schema for laminar burning velocity measurement datapoints | ||
| laminar-burning-velocity-measurement-schema: &laminar-burning-velocity-measurement-schema | ||
| type: list | ||
| minlength: 1 | ||
| schema: | ||
| type: dict | ||
| schema: | ||
| pressure: *value-unit-required | ||
| temperature: *value-unit-required | ||
| laminar-burning-velocity: *value-unit-required | ||
| pressure-rise: *value-unit-optional | ||
| composition: *composition | ||
| equivalence-ratio: | ||
| type: float | ||
| min: 0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reactor-volumewas added to the schema/common-properties andvalidation.property_units, butDataPoint.value_unit_propsdoesn’t include it. As a result, anyreactor-volumevalues present in YAML will be ignored when constructingDataPointobjects (information loss), and downstream code can’t access it as a parsedpint.Quantity. Addreactor-volumetovalue_unit_props(and ensure it’s handled consistently with the other value+unit properties).