-
-
Notifications
You must be signed in to change notification settings - Fork 572
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
Add single-layer SEI #2333
Comments
I would like to work on this. Can you please assign this issue to me? |
Thanks @iatzak ! Let's work through required changes here before you write any code. If you have ideas on what to do please write out your plan here. If you're unsure or have any questions feel free to ask |
Great, thanks for the availability to answer questions! I'm attending a course where the students are supposed to contribute to an open-source simulation software, and I have chosen PyBaMM. Until the Christmas break I'll be working on a report about PyBAMM's "infrastructure" (CI / documentation / building / contribution / git workflow). After the break, I'll get to this issue, firstly by writing here what I plan to do, as you suggested. |
That's awesome! If you can share the report or make it public, let us know what can be better, and contribute to making it better, that would be really helpful |
Sure! :) I'll contribute with what I can. I should talk to the professor about sharing the report, but I see no reason to be prohibited. |
Hi @iatzak, how's it going? We are running a hackathon on 29th of March in Oxford and this issue is potentially one we could work on together. You'd be welcome to join us if you can make it. Tickets are free but limited availability https://www.eventbrite.co.uk/e/pybamm-hackathon-tickets-517334430207?keep_tld=1 |
Hi @TomTranter, thanks for the heads up. I'm working on this later than I expected due to university courses, however I still plan to contribute to this issue. @tinosulzer could you describe shortly in which files the changes need to be made? |
@iatzak Documentation for current SEI models |
No need to create a new python file. Just use the same file and read in the option "number of SEI layers" (which you'll have to add to |
Hey @iatzak, I really would like to work on this issue, let me know if you are still busy. Thanks. |
Hi @jeromtom, you can go ahead, since I made my course-related contribution on another issue and I would only start working on this one in the next week. |
Hi @tinosulzer, I would like to work on this issue, could you please assign it to me? |
Currently the default for SEI is to have an inner and outer layer. This is only for historical reasons because Scott Marquis was investigating this in his [thesis].
In practice most people set both layers to be equal or set the inner layer to be zero. It would be good to have the option to only have a single layer SEI. This should probably be the default.
The text was updated successfully, but these errors were encountered: