-
Notifications
You must be signed in to change notification settings - Fork 56
docs: example on battery pack [skip tests] #4596
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?
Conversation
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.
Pull Request Overview
This PR adds two new PyFluent example workflows demonstrating electrochemical simulations: a PEM electrolyzer model and a 1P3S battery pack simulation. Both examples follow the new API implementation guidelines and provide end-to-end workflows with visualization.
- Introduces
Electrolysis_Modeling_workflow.pydemonstrating PEM electrolyzer simulation with Butler-Volmer kinetics - Adds
Battery_Pack.pyshowing NTGK battery pack modeling with thermal management - Updates documentation configuration and CI workflow to include the new examples
Reviewed Changes
Copilot reviewed 4 out of 16 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| examples/00-fluent/Electrolysis_Modeling_workflow.py | New example implementing PEM electrolysis simulation with multiphase flow and electrochemical reactions |
| examples/00-fluent/Battery_Pack.py | New example demonstrating 1P3S battery pack simulation with NTGK model and dual-path conductivity |
| doc/source/conf.py | Added new example filenames to the Sphinx gallery configuration pattern |
| .github/workflows/execute-examples-weekly.yml | Added execution steps for both new examples in the weekly CI workflow |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove Electrolysis_Modeling_workflow.py - Remove electrolysis related PNG images - Keep only battery pack example files
200f190 to
5ff8c9f
Compare
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.
Pull Request Overview
Copilot reviewed 3 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
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.
Pull Request Overview
Copilot reviewed 4 out of 12 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
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.
Pull Request Overview
Copilot reviewed 4 out of 12 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@MohammedAnsys, please re-run the pre-commit from your end to fix the code styling issues. |
Done! re-ran pre-commit and fixed all styling issues. |
Co-authored-by: Abhishek Chitwar <[email protected]>
Co-authored-by: Abhishek Chitwar <[email protected]>
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.
Pull Request Overview
Copilot reviewed 4 out of 13 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Use more descriptive variable names (zone, wall instead of z, w) - Use descriptive report definition names (voltage_surface_areaavg, volume_max_temp) - Improve list comprehension formatting for better readability - Fix spelling: utlizing -> utilizing - Maintain consistent naming conventions throughout the code
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.
Pull Request Overview
Copilot reviewed 4 out of 13 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Sean Pearson <[email protected]>
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.
Pull Request Overview
Copilot reviewed 4 out of 13 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Simulation of a 1P3S lithium-ion battery pack, consisting of three cells | ||
| # connected in series and no parallel branches, undergoing a constant 200 W | ||
| # discharge to represent a high-load condition typical in electric vehicle | ||
| # operations such as acceleration or regenerative braking. Each cell has a |
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.
Is this described correctly? It seems to associate regenerative braking with battery discharge.
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.
File name doesn't fit the naming convention.
| # operations such as acceleration or regenerative braking. Each cell has a | ||
| # nominal capacity of 14.6 Ah, resulting in a total pack capacity of 14.6 Ah | ||
| # and a nominal voltage range of 10.8–12.6 V, corresponding to an average cell | ||
| # voltage of approximately 3.9 V. The discharge rate equates to about 1.17 C, |
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.
Omit the space in C-rate values. Otherwise, the reader could misinterpret them. To be even clearer, we might write something like "a C-rate of 1.17C" in place of "1.17 C".
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.
Pull Request Overview
Copilot reviewed 4 out of 13 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull Request Overview
Copilot reviewed 4 out of 13 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # ------------------------------------- | ||
|
|
||
| # Get all solid cell zones | ||
| all_cell_zones = solver.settings.setup.cell_zone_conditions.solid.get_object_names() |
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.
CellZoneConditions has been imported
| all_cell_zones = solver.settings.setup.cell_zone_conditions.solid.get_object_names() | ||
|
|
||
| # Get all wall boundaries | ||
| all_wall_names = solver.settings.setup.boundary_conditions.wall.get_object_names() |
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.
Ditto
| all_wall_names = solver.settings.setup.boundary_conditions.wall.get_object_names() | ||
|
|
||
| # CELLS | ||
| cell_zones = [zone for zone in all_cell_zones if "cell_" in zone] |
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.
Could you have used wildcard syntax to filter the names directly via the API?
| save_path=os.getcwd(), | ||
| ) | ||
|
|
||
| solver.settings.file.read_mesh(file_name=mesh_file) |
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.
Just as you can import objects, you can also import commands like ReadMesh.
|
|
||
| battery_model.enabled = True | ||
| battery_model.echem_model = "ntgk/dcir" | ||
| battery_model.eload_condition.eload_settings.eload_type = "specified-system-power" |
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 API design follows a peculiar hierarchy here, by prefixing with eload at three levels.
End-to-end PyFluent workflow on battery pack
ANSYS Fluent User's Guide, ANSYS, Inc.
Simulating a 1P3S Battery Pack Using the Battery Model
The example tries to follow the provided guidelines with #4378 and uses the new API implementation.
Thanks.