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

Duplicate initial gas property in python example ic_engine.py #1460

Closed
flight-test-engineering opened this issue Mar 21, 2023 · 3 comments · Fixed by #1515
Closed

Duplicate initial gas property in python example ic_engine.py #1460

flight-test-engineering opened this issue Mar 21, 2023 · 3 comments · Fixed by #1515

Comments

@flight-test-engineering
Copy link

flight-test-engineering commented Mar 21, 2023

Gas properties are defined 2 times in ic_engine.py

This is just a documentation issue.

Line gas.TPX = T_inlet, p_inlet, comp_inlet is duplicated:

...
#####################################################################
# Set up Reactor Network
#####################################################################

# load reaction mechanism
gas = ct.Solution(reaction_mechanism, phase_name)

# define initial state and set up reactor
gas.TPX = T_inlet, p_inlet, comp_inlet
cyl = ct.IdealGasReactor(gas)
cyl.volume = V_oT

# define inlet state
gas.TPX = T_inlet, p_inlet, comp_inlet
inlet = ct.Reservoir(gas)
...

Suggest to remove duplicate.

@bryanwweber
Copy link
Member

Thanks for reporting this! Despite appearances, I believe this is intended. The reason is that in principle, the reservoir can have a different state than the reactor (cylinder). As such, we are demonstrating that you can set those values before creating the reservoir, even on the same object, without changing the reactor.

However, a note to that effect is probably warranted! Thanks ☺️

@flight-test-engineering
Copy link
Author

Thanks for responding. Yes, it makes sense. Should I close the issue?

@bryanwweber
Copy link
Member

No, I think we can leave it open, with the fix being a comment describing what's going on. If you'd like to open a pull request with the change, please feel free!

ischoegl added a commit to ischoegl/cantera that referenced this issue Jun 26, 2023
@ischoegl ischoegl mentioned this issue Jun 26, 2023
5 tasks
ischoegl added a commit to ischoegl/cantera that referenced this issue Jun 30, 2023
ischoegl added a commit to ischoegl/cantera that referenced this issue Jul 1, 2023
speth pushed a commit that referenced this issue Jul 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants