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

Fixes to reduce lband of cathode #78

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

mglasser16
Copy link
Contributor

This pull request is a work in progress, mostly creating it for discussion about improving the code in our 1v1. Not intended for edits to be added to batcan.

Copy link
Contributor

@decaluwe decaluwe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pushing this - looking forward to discussing, later!

@@ -55,6 +55,8 @@ def __init__(self, input_file, inputs, sep_inputs, counter_inputs,

# Phase volume fractions
eps_host = np.array([inputs['eps_host']])
eps_host = eps_host[0]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noting this line, which overwrites 57, so we can discuss it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

@@ -219,7 +220,7 @@ def residual(self, t, SV, SVdot, sep, counter, params):
c_k_elyte = SV_loc[SVptr['C_k_elyte'][j]]
eps_product = SV_loc[SVptr['eps_product'][j]]
eps_elyte = 1. - eps_product - self.eps_host[j]

self.elyte_microstructure = eps_elyte**1.5
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to above, this duplicates/overwrites line 98.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, line 98 is necessary to initialize for the separator, however self.elyte_microstructure needs to be updated for each node while the code is running.

electrode_models/air_electrode.py Outdated Show resolved Hide resolved
@@ -413,6 +414,9 @@ def residual(self, t, SV, SVdot, sep, counter, params):
+ sdot_elyte_host * A_surf_ratio)
* self.dyInv)/eps_elyte

eps_elyte = 1. - eps_product - self.eps_host[0]
self.elyte_microstructure = eps_elyte**1.5
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know that these lines do anything...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resets node to zero, because it is passed to separator as the final node.

@mglasser16
Copy link
Contributor Author

@decaluwe The new commits should be mergeable now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants