Transitional handling of 'gri30_mix' and 'gri30_multi' phases in YAML

[ischoegl]

Please fill in the issue number this pull request is fixing:

Fixes #736

Changes proposed in this pull request:

• catch gri30_mix and gri30_multi phase definitions for YAML and issue warnings
• replace occurrences of gri30.cti and gri30.xml By gri30.yaml in test suite

An attempt to load deprecated phases from gri30.yaml has the expected behavior, but issues a warning:

In [1]: import cantera as ct

In [2]: gas = ct.Solution('gri30.yaml', 'gri30_mix')
DeprecationWarning: The phases 'gri30_mix' and 'gri30_multi' are no longer defined in the input file 'gri30.yaml'. The YAML version uses the 'Mix' transport manager by default instead, which can be overridden by specifying a transport manager via the `transport_model' keyword argument during initialization. Support for transitional behavior will be removed after Cantera 2.5.

In [3]: gas.name, gas.composite
Out[3]: ('gri30_mix', ('IdealGas', 'Gas', 'Mix'))

In [4]: gas = ct.Solution('gri30.yaml', 'gri30_multi')
DeprecationWarning: The phases 'gri30_mix' and 'gri30_multi' are no longer defined in the input file 'gri30.yaml'. The YAML version uses the 'Mix' transport manager by default instead, which can be overridden by specifying a transport manager via the `transport_model' keyword argument during initialization. Support for transitional behavior will be removed after Cantera 2.5.

In [5]: gas.name, gas.composite
Out[5]: ('gri30_multi', ('IdealGas', 'Gas', 'Multi'))

[codecov]

Codecov Report

Merging #737 into master will increase coverage by <.01%.
The diff coverage is 67.85%.

@@            Coverage Diff             @@
##           master     #737      +/-   ##
==========================================
+ Coverage    71.4%   71.41%   +<.01%     
==========================================
  Files         372      372              
  Lines       43859    43878      +19     
==========================================
+ Hits        31317    31334      +17     
- Misses      12542    12544       +2

Impacted Files	Coverage Δ
test/equil/equil_gas.cpp	91.03% <100%> (ø)	⬆️
test_problems/clib_test/clib_test.c	100% <100%> (ø)	⬆️
src/transport/TransportFactory.cpp	100% <100%> (ø)	⬆️
src/thermo/ThermoFactory.cpp	81.73% <50%> (-1.75%)	⬇️
src/kinetics/KineticsFactory.cpp	93.42% <75%> (-1.1%)	⬇️
src/thermo/HMWSoln.cpp	71.54% <0%> (-0.05%)	⬇️
src/base/AnyMap.cpp	87.65% <0%> (+1.21%)	⬆️
include/cantera/thermo/ThermoPhase.h	28.66% <0%> (+1.33%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c4c30e3...de6ba7d. Read the comment docs.

[bryanwweber]

I think this solution is not surgical enough at the moment. The main problem I foresee is users who deliberately create their own version of GRI-3.0 with these phase definitions in them. I'm always surprised when I see someone on the Users' Group who has a copy of gri30.cti in their working directory, but as I recall it has happened multiple times. Is there any way to have this warning raised if and only if the gri30.yaml file comes from the installed data location?

[ischoegl]

@bryanwweber ... thank you for the review!

Regarding your comment about users potentially keeping local copies of gri30.ctior gri30.xml: this is implicitly taken care of, as the warning is only issued if users attempt to load a yaml file.

In [2]: gas = ct.Solution('gri30.yaml', 'gri30_mix')
/usr/bin/ipython3:1: DeprecationWarning: The phases named 'gri30_mix' and 'gri30_multi' are no longer defined in the input file 'gri30.yaml'. By default, 'gri30.yaml' uses the mixture-averaged transport model. The following options are available:
 * ct.Solution('gri30.yaml')  # Mixture-averaged transport
 * ct.Solution('gri30.yaml', transport_model='mix')  # Mixture-averaged transport
 * ct.Solution('gri30.yaml', transport_model=None)  # No transport model, useful for 0-D simulations
 * ct.Solution('gri30.yaml', transport_model='multicomponent')  # Multicomponent transport
Support for transitional behavior will be removed after Cantera 2.5.

In [3]: gas = ct.Solution('gri30.cti', 'gri30_mix') # does not issue a warning

In [4]: gas = ct.Solution('gri30.xml', 'gri30_mix') # does not issue a warning

However, you are correct that the solution needs to be more surgical: a custom yaml file may contain valid gri30_mix definitions, i.e. they should be used when present. A case in point is anything converted using cti2yaml or ctml2yaml ... which applies to gri30_highT.yaml etc.

Also: unit tests appear to load from test/work/python/, which contains a converted gri30.yaml (this causes the unit test for the deprecation warning to fail: at the moment it is commented out). Any thoughts on this (the simplest solution is to eliminate the test)?

[ischoegl]

@bryanwweber ... I believe it's now all fixed (and scons test-matlab is confirmed as I'm unsure that it is available on AppVeyor). FYI, on MATLAB the warning is:

>> gas = Solution('gri30.yaml','gri30_mix')
DeprecationWarning: newPhase: 
The phases named 'gri30_mix' and 'gri30_multi' are no longer defined in the input file 'gri30.yaml'. By default, 'gri30.yaml' uses the mixture-averaged transport model. The following options are available:
 * 'Mix': Mixture-averaged transport
 * 'Multi': Multicomponent transport
 * 'None': No transport model, useful for 0-D simulations
Support for transitional behavior will be removed after Cantera 2.5.

It's hard to make it more specific as the warning is raised from the C++ layer (i.e. I cannot refer to MATLAB specific syntax). As an aside, the None option is newly added to MATLAB ...

[bryanwweber]

For the Python interface, at least, it doesn't appear that the appropriate transport model is set (I could be wrong, I'm not sure if there's an extra step that calls into C++ that I'm missing in the diff view here). If a user is passing 'gri30_multi' they would expect that Multi-component transport is enabled, but they'll get an object with mixture-averaged transport. Can that be resolved?

[bryanwweber]

and scons test-matlab is confirmed as I'm unsure that it is available on AppVeyor

MATLAB can't be tested on any of the public, free CI services because we can't install MATLAB on them. @speth has a buildbot instance set up on some hardware at MIT that runs the (anemic) MATLAB test suite IIRC, but I can't recall the URL for that, or if it's public facing.

[ischoegl]

If a user is passing 'gri30_multi' they would expect that Multi-component transport is enabled, but they'll get an object with mixture-averaged transport. Can that be resolved?

I cannot reproduce this:

n [1]: import cantera as ct

In [2]: gas = ct.Solution('gri30.yaml', 'gri30_multi')
/usr/bin/ipython3:1: DeprecationWarning: The phases named 'gri30_mix' and 'gri30_multi' are no longer defined in the input file 'gri30.yaml'. By default, 'gri30.yaml' uses the mixture-averaged transport model. The following options are available:
 * ct.Solution('gri30.yaml')  # Mixture-averaged transport
 * ct.Solution('gri30.yaml', transport_model='mix')  # Mixture-averaged transport
 * ct.Solution('gri30.yaml', transport_model=None)  # No transport model, useful for 0-D simulations
 * ct.Solution('gri30.yaml', transport_model='multicomponent')  # Multicomponent transport
Support for transitional behavior will be removed after Cantera 2.5.
  #! /bin/sh

In [3]: gas.composite
Out[3]: ('IdealGas', 'Gas', 'Multi')

[ischoegl]

Regarding MATLAB:

>> gas = Solution('gri30.yaml','gri30_multi');
DeprecationWarning: newPhase: The phases named 'gri30_mix' and 'gri30_multi' are no longer defined in the input file 'gri30.yaml'. By default, 'gri30.yaml' uses the mixture-averaged transport model. The following options are available:
 * 'Mix': Mixture-averaged transport
 * 'Multi': Multicomponent transport
 * 'None': No transport model, useful for 0-D simulations
Examples for setting of transport model:
 - MATLAB: Solution('gri30.yaml', 'gri30', 'None')
 - C++: newSolution("gri30.yaml", "gri30", "Mix")
Support for transitional behavior will be removed after Cantera 2.5.
>> multi = multiDiffCoeffs(gas); % does not throw error

and, in the same session (warning is only shown the first time)

>> gas = Solution('gri30.yaml','gri30_mix');
>> multi = multiDiffCoeffs(gas);
Error using ctmethods
[...]

[bryanwweber]

If a user is passing 'gri30_multi' they would expect that Multi-component transport is enabled, but they'll get an object with mixture-averaged transport. Can that be resolved?

I cannot reproduce this:

Thanks for checking! That's what I hoped would happen but couldn't tell from the diff view 😄

[ischoegl]

Thanks for checking! That's what I hoped would happen but couldn't tell from the diff view smile

You're welcome. For these exceptions, the transport model simply goes by phase name (which doesn't change), and the conversion is caught elsewhere. (Transport models get instantiated much later.) I actually added two lines to the unit tests to make sure that this behavior won't change ...

[bryanwweber]

@ischoegl What about this warning message?

            if name in ['gri30_mix', 'gri30_multi']:
                input_file = pystr(root[stringify("__file__")].asString())
                # catch transitional behavior
                warnings.warn(dedent("""
                    The phase named '{}' is not defined in the input file '{}'.
                    Examples that use this phase name are deprecated. Instead, you
                    should use one of the following options to choose a transport
                    model:

                    * ct.Solution("gri30.yaml")  # Mixture-averaged transport, default
                    * ct.Solution("gri30.yaml", transport_model="mix")  # Mixture-averaged transport
                    * ct.Solution("gri30.yaml", transport_model=None)  # No transport model, useful for 0-D simulations
                    * ct.Solution("gri30.yaml", transport_model="multicomponent")  # Multicomponent transport
                """.format(name, input_file)), DeprecationWarning)

Maybe add This warning will convert to an error after Cantera 2.5 at the end.

Rewriting this message, I went back and forth on whether user-generated input files should be handled in this way. On the one hand, it is nice for a user if they're following an example and the end up using one of these phase names and get warned about it. On the other hand, if I were that user, I'm probably very new to Cantera, and having warnings like this thrown at me is not super helpful.

FYI, asString() has to be added to _cantera.pxd for this to compile and you have to add from textwrap import dedent to the top of base.pyx.

One edge case I just thought of (this is a little convoluted, but bear with me 😄 ):

1. User creates their own gri30.yaml by converting the CK files
2. User changes the default phase name in the file, either during conversion or afterwards
3. User tries to load that file and passes one of these deprecated phase names
4. Kaboom!

In [3]: ct.Solution("./gri30.yaml", "gri30_mix")
<ipython-input-3-1e59bb5fc60e>:1: DeprecationWarning:
The phase named 'gri30_mix' is not defined in the input file './gri30.yaml'.
Examples that use these phases are deprecated.
Instead, you should use one of the following options:

* ct.Solution('gri30.yaml')  # Mixture-averaged transport
* ct.Solution('gri30.yaml', transport_model='mix')  # Mixture-averaged transport
* ct.Solution('gri30.yaml', transport_model=None)  # No transport model, useful for 0-D simulations
* ct.Solution('gri30.yaml', transport_model='multicomponent')  # Multicomponent transport

  ct.Solution("./gri30.yaml", "gri30_mix")
---------------------------------------------------------------------------
CanteraError                              Traceback (most recent call last)
~/GitHub/cantera/interfaces/cython/cantera/base.pyx in cantera._cantera._SolutionBase._init_yaml()
    113         try:
--> 114             phaseNode = root[stringify("phases")].getMapWhere(
    115                 stringify("name"), stringify(name))

CanteraError:
***********************************************************************
InputFileError thrown by AnyValue::getMapWhere:
Error on line 16 of ./gri30.yaml:
List does not contain a map where 'name' = 'gri30_mix'
|  Line |
|    11 | date: Fri, 13 Dec 2019 16:19:11 -0500
|    12 |
|    13 | units: {length: cm, time: s, quantity: mol, activation-energy: cal/mol}
|    14 |
|    15 | phases:
>    16 > - name: gas
          ^
|    17 |   thermo: ideal-gas
|    18 |   elements: [O, H, C, N, Ar]
|    19 |   species: [H2, H, O, O2, OH, H2O, HO2, H2O2, C, CH, CH2, CH2(S), CH3, CH4,
***********************************************************************


During handling of the above exception, another exception occurred:

CanteraError                              Traceback (most recent call last)
<ipython-input-3-1e59bb5fc60e> in <module>
----> 1 ct.Solution("./gri30.yaml", "gri30_mix")

~/GitHub/cantera/interfaces/cython/cantera/base.pyx in cantera._cantera._SolutionBase.__cinit__()
     54         # Parse inputs
     55         if infile.endswith('.yml') or infile.endswith('.yaml') or yaml:
---> 56             self._init_yaml(infile, name, adjacent, yaml)
     57         elif infile or source:
     58             self._init_cti_xml(infile, name, adjacent, source)

~/GitHub/cantera/interfaces/cython/cantera/base.pyx in cantera._cantera._SolutionBase._init_yaml()
    128                     * ct.Solution("gri30.yaml", transport_model=None)  # No transport model, useful for 0-D simulations
    129                     * ct.Solution("gri30.yaml", transport_model="multicomponent")  # Multicomponent transport
--> 130                 """.format(name, input_file)), DeprecationWarning)
    131                 phaseNode = root[stringify("phases")].getMapWhere(
    132                     stringify("name"), stringify("gri30"))

CanteraError:
***********************************************************************
InputFileError thrown by AnyValue::getMapWhere:
Error on line 16 of ./gri30.yaml:
List does not contain a map where 'name' = 'gri30'
|  Line |
|    11 | date: Fri, 13 Dec 2019 16:19:11 -0500
|    12 |
|    13 | units: {length: cm, time: s, quantity: mol, activation-energy: cal/mol}
|    14 |
|    15 | phases:
>    16 > - name: gas
          ^
|    17 |   thermo: ideal-gas
|    18 |   elements: [O, H, C, N, Ar]
|    19 |   species: [H2, H, O, O2, OH, H2O, HO2, H2O2, C, CH, CH2, CH2(S), CH3, CH4,
***********************************************************************

Anyways, this sequence of events should raise an error (because the name they passed really isn't defined), so I'm not sure why the user would expect it to work in the first place. And it seems pretty unlikely to happen. Just thought I'd throw it out there.

[ischoegl]

@bryanwweber ... thanks for the suggestion. Will adopt.

Otherwise, interesting scenario! Unfortunately, this is even raised if the phase name is not changed in ck2yaml, so it's not that unlikely. I believe there is a way to just have the default be used (the first phase specified in the file), which would be the safe thing to do. Thanks for catching this!

[speth]

If we are effectively making the data files part of the API, where changes are subject to deprecation warnings, then it might be worth discussing a more general approach to making such changes, rather than a one-off for just the gri30_multi and gri30_mix phase definitions. I'm thinking of this in terms of the comment I made on #768, where I noted that I'd like to remove (or deprecate and later remove) some of the currently-included files.

One possible implementation would be to add a field to either the file or the phase definition that is checked for when the file is loaded, and used to issue a deprecation warning. This would have the benefit of not affecting copies of files (modified or not) that users have placed on their system outside the Cantera data directories. For the YAML format, a file-level deprecation could be written:

generator: ctml2yaml
cantera-version: 2.5.0a3
date: Thu, 21 Nov 2019 19:34:25 -0500
input-files: [build\data\gri30.xml]
deprecated: >
  The input file silicon_carbide.yaml is deprecated and will be removed
  after Cantera X.Y

Or an individual phase definition could be deprecated by adding:

phases:
- name: gri30_mix
  thermo: ideal-gas
  transport: mixture-averaged
  kinetics: gas
  reactions: all
  state: {T: 300.0 K, P: 1.01325e+05 Pa}
  deprecated: >
    The gri30_mix phase definition will be removed from gri30.yaml
    after Cantera X.Y.

In CTI, there could be a top-level deprecated directive that acts on the file. For individual phases, we could put this into the options field, e.g.

ideal_gas(name = "gri30_mix",
      elements = " O  H  C  N  Ar ",
      species = """ H2  H  O  O2  OH  H2O  HO2  H2O2  C  CH 
                   CH2  CH2(S)  CH3  CH4  CO  CO2  HCO  CH2O  CH2OH  CH3O 
                   CH3OH  C2H  C2H2  C2H3  C2H4  C2H5  C2H6  HCCO  CH2CO  HCCOH 
                   N  NH  NH2  NH3  NNH  NO  NO2  N2O  HNO  CN 
                   HCN  H2CN  HCNN  HCNO  HOCN  HNCO  NCO  N2  AR  C3H7 
                   C3H8  CH2CHO  CH3CHO """,
      reactions = "all",
      transport = "Mix",
      options = ["deprecated: The gri30_mix phase definition will be removed from gri30.cti"
                 " after Cantera X.Y."]
      initial_state = state(temperature = 300.0,
                        pressure = OneAtm)    )

[ischoegl]

@speth ... I think adding deprecation tags is a good idea. The question boils down to: Would you suggest to ditch the ck2yaml route for gri30.yaml in favor of ctml2yaml? It’s a legitimate approach, albeit different from this PR.

The one issue that would remain is that the default phase for gri30.yaml does not have a transport model, and thus is not compatible with default behavior of auto-generated yaml files, even if deprecation tags are used to eliminate other superfluous phase entries. Overall, I’d still advocate for a consistent default (e.g. mixture-averaged transport if transport data are available), which i believe does require some input-specific transitional handling as this PR proposes.

[bryanwweber]

thus is not compatible with default behavior of auto-generated yaml files

I'm not sure what you mean by auto-generated, but it's perfectly fine to have a YAML file without transport data.

i believe does require some input-specific transitional handling as this PR proposes.

I don't think that would be necessary. If the default changes from no transport to mixture-averaged, I don't see a problem with that, or a need to warn anyone. IMO the one advantage this approach has is showing a way to turn off transport and reduce the time it takes to load the file, especially for large mechanisms where the only real possibility is zero-D simulations anyways. But we could add that to an example

[ischoegl]

@bryanwweber ... to clarify

CTI/XML offer the following phases

gri30: no transport # <-- loaded by default if no phase is specified
gri30_mix: mixture-averaged transport
gri30_multi:. multi-compon transport

The (current) gri30.yaml generated by ck2yaml is incompatible (and eventually led to this PR), as it has a single phase

gri30: mixture-averaged transport # <-- loaded by default

Note that any ck2yaml generated input file will have a transport model loaded by default if data are available (which is where gri30.cti differs).

As indicated, the question boils down to whether:

1. the old behavior should be preserved, i.e. do not load transport data. (Solution: create gri30 via cti2yaml; and use deprecation tags) ... completely separate from this PR
2. gri30.yaml should exhibit the regular behavior expected from input generated from ck2yaml if transport data are included (i.e. load transport data if no phase is specified) ... this PR in its current form.
3. There are other options, e.g. (a) change default loading behavior to no transport model, or (b) add a flag to ck2yaml that specifies what transport model is attached to the single phase that is created

If the consensus is to move away from (2), I'd suggest closing this PR and starting a new one as none of this PR's code carries over.

[speth]

The one issue that would remain is that the default phase for gri30.yaml does not have a transport model, and thus is not compatible with default behavior of auto-generated yaml files, even if deprecation tags are used to eliminate other superfluous phase entries. Overall, I’d still advocate for a consistent default (e.g. mixture-averaged transport if transport data are available), which i believe does require some input-specific transitional handling as this PR proposes.

Making gri30.yaml consistent with what a user would end up generating from their own Chemkin input files was the main point of removing the custom phase definitions of gri30.cti. I agree that "mixture-averaged transport if transport data are available", which is the convention inherited from ck2cti is a reasonable default.

However, you are correct that the solution needs to be more surgical: a custom yaml file may contain valid gri30_mix definitions, i.e. they should be used when present. A case in point is anything converted using cti2yaml or ctml2yaml ... which applies to gri30_highT.yaml etc.

The logic that applies eliminating the alternate phase definitions from gri30 applies to gri30_highT as well. The only thing that makes this trickier is that gri30_highT.cti seems to be a manually-edited copy of gri30.cti, rather than generated Chemkin-format files. So the only way to implement that change will be to commit the desired version of gri30_highT.yaml directly, perhaps as part of #768.

One edge case I just thought of (this is a little convoluted, but bear with me ):

1. User creates their own gri30.yaml by converting the CK files
2. User changes the default phase name in the file, either during conversion or afterwards
3. User tries to load that file and passes one of these deprecated phase names
4. Kaboom!

This does indeed seem pretty convoluted, and I think the conditions are even more specific than this: the user would have to remove both the gri30 and (say) the gri30_mix phase definitions, but then try to load the gri30_mix phase definition. And the only thing wrong with the error message is that it specifies that the missing name is gri30 rather than gri30_mix.

[bryanwweber]

@speth Are you no longer in favor of the deprecated field in the YAML file? I think that would actually be quite useful; it could also be used to deprecate a certain, say, phase or species thermo type, if necessary:

phases:
- name: example
  thermo: something-deprecated
  deprecated: The 'something-deprecated' phase thermo type is deprecated...
  ...

Obviously, this would have to be added manually to any files that used that thermo type, and presumably there would be deprecation warnings when the thermo type was instantiated, but it would still be a useful field to have IMO.

If you would still like to pursue that avenue, I agree with @ischoegl that this PR should be closed. My preference is for suggestion (1) from @ischoegl, that is, retain the gri30, gri30_mix, and gri30_multi phases for 2.5.0, add the deprecation notice, and change all examples to explicitly specify the desired transport model and no longer use the phase names in our examples. Regardless of the outcome here, I'm planning to add the explicit transport_model option to the examples anyways as part of #768.

I think the conditions are even more specific than this

I think the more likely case is a user converts the GRI-3.0 CK files to YAML, resulting in the default name of gas in the file (at least, I think that would be the default). As such, it's not quite as convoluted as I originally thought (@ischoegl pointed out this case). To be clear, my concern wasn't so much with the specificity of the error message, but rather with its length (it specifies that both gri30 and gri30_mix are missing at different points). The most likely people to see that particular error message are probably relatively new users, and that would not be a particularly easy one to parse without some prior experience with Python tracebacks.

[ischoegl]

Making gri30.yaml consistent with what a user would end up generating from their own Chemkin input files was the main point of removing the custom phase definitions of gri30.cti.

That was my understanding before starting this PR, which led me to implementation (2) 😉. I personally agree with @speth that it's preferable to have consistent behavior. The switch of formats is imho an opportunity to get rid of inconsistent implementations.

I am not opposed to the deprecated flag whatsoever, but I believe it should be handled in another PR. It certainly is well aligned with #768, although it doesn't necessarily have to be addressed there.

PS: the convoluted error message can be fixed/avoided with fairly little effort.

[speth]

Are you no longer in favor of the deprecated field in the YAML file? I think that would actually be quite useful

No, I still think it would be useful, and I'd like to see it before we act on #768. Either the addition of a deprecated field in the input file or this PR would allow us to make the transition to a simpler version of gri30.yaml. The deprecated field would of course also be useful for several other situations. The one advantage of this approach would be that we wouldn't need to start of by introducing some deprecated phase definitions in gri30.yaml.

Perhaps we should put together an implementation of the deprecated field first and depending on how well that works we can decide whether that feature or the approach in this PR is the best way to deal with the gri30_mix / gri30_multi issue.

[ischoegl]

@speth and @bryanwweber ... I finished implementation (2), and made sure that the awkward error Bryan pointed out will not appear (there is a unit test for it).

@speth: Making gri30.yaml consistent with what a user would end up generating from their own Chemkin input files was the main point of removing the custom phase definitions of gri30.cti.

To reiterate: if the above is the objective (as had been my understanding), then there are two implementation paths for how users would upgrade existing custom models:

(1) the deprecated field:

• Cantera 2.5: shows a deprecation warning for YAML, but has behavior of old CTI files
• Cantera 2.6: default transport model loading behavior changes from None to Mix
• Cantera 3.0: all models are fixed

(2) this PR:

• Cantera 2.5: emulates old CTI behavior to facilitate transition to YAML
• Cantera 2.6: all models are fixed

Imho the main advantage of (2) is that there will be exactly one version of gri30.yaml (single phase), whereas for implementation (1), there will be two versions (three phase for 2.5 and single phase for 2.6). Also, while it's possible to fix official cantera examples immediately, user code may have to be changed twice for (1), whereas it needs to change once for (2). I am very sceptical about adopting superfluous phases in the newly created YAML format.

In summary, while I believe the deprecated flag makes a lot of sense elsewhere, a change of default behavior for gri30 requires more effort.

[ischoegl]

PS: the two version of the warnings

Python

In [2]: gas = ct.Solution("./gri30.yaml", "gri30_mix")
/usr/bin/ipython3:1: DeprecationWarning: 
The phase named 'gri30_mix' is not defined in the input file './gri30.yaml'.
Examples that use this phase name are deprecated. Instead, you should
use one of the following options to choose a transport model:

* ct.Solution("gri30.yaml")  # Mixture-averaged transport, default
* ct.Solution("gri30.yaml", transport_model="Mix")  # Mixture-averaged transport
* ct.Solution("gri30.yaml", transport_model=None)  # No transport model, useful for 0-D simulations
* ct.Solution("gri30.yaml", transport_model="Multi")  # Multicomponent transport

This warning will convert to an error after Cantera 2.5.

C++/Matlab version (uses a temporarily modified example for illustration purposes)

$ ./combustor
DeprecationWarning: newPhase: The phase named 'gri30_mix' is not defined in the input file '/usr/local/share/cantera/data/gri30.yaml'.
Examples that use this phase name are deprecated. Instead, you should specify one of the following transport models:
 * 'Mix': Mixture-averaged transport
 * 'Multi': Multicomponent transport
 * 'None': No transport model, useful for 0-D simulations
Examples:
 - MATLAB: Solution('gri30.yaml', 'gri30', 'None')
 - C++: newSolution("gri30.yaml", "gri30", "Mix")
This warning will convert to an error after Cantera 2.5.

[bryanwweber]

@ischoegl: have to be changed twice for (1), whereas it needs to change once for (2). ... In summary, while I believe the deprecated flag makes a lot of sense elsewhere, a change of default behavior for gri30 requires more effort.

I don't necessarily think this is true. I don't see a problem with changing the default phase in gri30.yaml to mixture-averaged transport and also retaining the gri30_mix phase. I can't think of any cases where changing the default to include transport would create a problem (certainly, the reverse is not true).

@speth: The one advantage of this approach [in PR 737, this one] would be that we wouldn't need to start of by introducing some deprecated phase definitions in gri30.yaml.

This is true, but I don't think it's that big of an advantage. If we are going to be removing/changing other input files after using the deprecated field, we might as well do all the breaking changes at once, including gri30.yaml.

Another thought... From the warning message:

* ct.Solution("gri30.yaml")  # Mixture-averaged transport, default

I know that I wrote this message, but I keep thinking of ways it could be misinterpreted. For this line, this is only definitely the default for the built-in gri30.yaml. If a user converts the CK GRI-3.0 files themselves and doesn't include transport (not unreasonable, I rarely include transport data if I know I'm going to just be doing 0-D simulations), the warning message will be wrong.

There are just so many edge cases with trying to guess what a user meant to do with their own input files, I just can't shake the feeling that this warning should be limited only to files distributed with Cantera. 😞 Sorry to put this PR through the ringer, @ischoegl thanks for the constructive discussion!

[ischoegl]

@bryanwweber ... I’m open for suggestions/clarifications. Getting the warning message right now should pay off later.

[ischoegl]

Another comment: rather than porting CTI to YAML and immediately adding a deprecated field, wouldn't it preferable to add a deprecated tag and warning to CTI and never even include deprecated phases/input in YAML?

[ischoegl]

Closing as solution proposed in #768 is less intrusive and able to handle other cases as well.