Open editorialbot opened 5 months ago
Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.
For a list of things I can do to help you, just type:
@editorialbot commands
For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
@editorialbot generate pdf
Software report:
github.com/AlDanial/cloc v 1.90 T=0.05 s (1133.1 files/s, 146764.4 lines/s)
-------------------------------------------------------------------------------
Language files blank comment code
-------------------------------------------------------------------------------
MATLAB 53 801 2888 2933
Markdown 2 62 0 406
TeX 1 24 0 246
YAML 1 1 4 18
-------------------------------------------------------------------------------
SUM: 57 888 2892 3603
-------------------------------------------------------------------------------
Commit count by author:
200 COBRAPROsimulator
1 CO-simulation BatteRy modeling for Accelerated PaRameter Optimization (COBRAPRO)
Paper file info:
📄 Wordcount for paper.md
is 2632
✅ The paper includes a Statement of need
section
License info:
✅ License found: MIT License
(Valid open source OSI approved license)
:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
👋🏼 @yuefan98 @BradyPlanden @brosaplanella, this is the review thread for the paper. All of our communications will happen here from now on.
As a reviewer, the first step is to create a checklist for your review by entering
@editorialbot generate my checklist
as the top of a new comment in this thread.
These checklists contain the JOSS requirements. As you go over the submission, please check any items that you feel have been satisfied. The first comment in this thread also contains links to the JOSS reviewer guidelines.
The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention openjournals/joss-reviews/issues/6803
so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread.
We aim for reviews to be completed within about 4-6 weeks. Please feel free to ping me (@mbarzegary) if you have any questions/concerns.
@editorialbot check references
@COBRAPROsimulator this is where the review takes place. Please keep an eye out for comments here from the reviewers, as well as any issues opened by them on your software repository. I recommend you aim to respond to these as soon as possible, and you can address them straight away as they come in if you like, to ensure we do not loose track of the reviewers.
@COBRAPROsimulator It seems there is an issue with the ARM version of MATLAB compatibility. Was able to run code successfully with the intel version of MATLAB. I have created an issue on target repo
https://github.com/COBRAPROsimulator/COBRAPRO/issues/1
@COBRAPROsimulator Overall, this is a great work that requires significant effort. I have several comments that I would like to get clarified and some features that I would like to be considered for future release.
Given the code seems to run forever on my side, I want to confirm first that the code is expected to return SOC dependent estimation of physical parameters for the DFN model rather than a single set of parameters that can fit the HPPC data for the whole SOC range.
@yuefan98, thank you very much for reviewing COBRAPRO! Here are the answers to your questions:
Please let me know if there are any further questions or points I can clarify.
@COBRAPROsimulator Thanks for the clarifications!
For 1. I am good with that unless other reviewers have comments on it. For 2. I think that make sense to me. For 4. I was able to validate your HPPC result by reducing it to two particles optimization. I think the estimation provides reasonable reaction rate given the fit to the instantaneous response. After reviewing the simulation in detail, I am a bit worry about the diffusion properties in the optimization. And it seems we generally have a faster solids state diffusion estimation than the actual battery response. Can you comment on the diffusion coefficient estimation? That is assumed to be constant over SOC right? Do you think the estimation can be improve by considered SOC dependence in the future or it is more a feature of model?
Lastly, I am glad that SOC dependence optimization is considered as a future work. I believe that will make this toolbox more useful for the real application.
Lastly, we plan to link our recently accepted paper from the Journal of The Electrochemical Society which explains COBRAPRO's numerical implementation and parameter identification framework in detail.
I think that will be really helpful !
@yuefan98, thank you. Here is my comment to 4:
I hope I answered your question. Please let me know if you have any other questions.
@yuefan98 thanks a lot for finalizing your review. @BradyPlanden how is your review going?
Here's my review. Overall, the package provides a novel solution to the parameterisation of physics-based battery models (in this case the DFN). The code is functional and the article well-written, and meets all the points in the checklist (though note I have some questions regarding the performance and testing, see below). I provide a list of comments to address before publication, split into major and minor comments.
cycle_CC.m
it took around 8 seconds (the PyBaMM time in my machine is similar to that reported by the authors). I understand that the discrepancy is probably due to pre/post-processing and printing to screen, but it would be good to have one example with minimal overhead which can demonstrate the performance claimed in the article.DFN_LSA_Corr_CC.m
) does not render LaTeX.DFN_pso_0_05C.m
also took a long time to me (around 30 minutes) and some errors/warning came up. It would be good to clarify in the documentation that this example might take a while to run.@BradyPlanden how is your review going?
@COBRAPROsimulator can you please provide an update on the above, in terms of responding to the issues raised by @brosaplanella?
Hi @mbarzegary and @brosaplanella, I sincerely apologize for the delay. I will reply to @brosaplanella's comments soon. Thank you for your understanding.
Apologies for the delay, here is my review in full.
Overall, this paper is well written and makes a novel contribution to the subfield of parameterisation for electrochemical battery modelling. As the current work has all been completed through the @COBRAPROsimulator account, I have assumed that is the first author, as Ferran mentioned above, setting up a contributor list would be helpful and enable expansion in the future.
@COBRAPROsimulator can you please provide an update on the raised issues? We need to move forward with this submission.
Hi @mbarzegary, I apologize for the delay. I will reply to the reviewers comments now. There was some delay due to my PhD thesis defense. Thank you for your understanding.
@brosaplanella , thank you so much for your detailed review. I apologize for my delayed response. Here are my responses below:
Major comments:
Comment 1. One of the main claims in the paper is performance, and it is stated that the 1C discharge runs in less than a second (similar order of magnitude to PyBaMM). When running it on my laptop (it tic/toced the first 1C discharge in cycle_CC.m it took around 8 seconds (the PyBaMM time in my machine is similar to that reported by the authors). I understand that the discrepancy is probably due to pre/post-processing and printing to screen, but it would be good to have one example with minimal overhead which can demonstrate the performance claimed in the article.
Answer 1. Thank you for your comment. In the cycle_CC.m file, the "[output,param] = runModel" function calculates output, which is a structure that reports the time it look to solve the DFN model. The exact time is reported as "output.DAE_solve_time" in the "output" structure. This calculates the pure time it took to solve the DFN model (not including printing and code overhead). I can modify the cycle_CC.m file so that it also prints the time to solve the DFN model. This should be less than one second for one cycle of 1C discharge for Np=Ns=Nn=Nrp=Nrn=10. We conducted a systematic computation study in our accepted JES paper which will be published soon.
Comment 2: I agree with @yuefan98 that the documentation is a bit sparse. Even though for a research tool the current documentation might be enough, it should be expanded in the near future (I am also happy for this to take place after publication). I believe the main issue is that, even though the examples work straight out of the box, it would be extremely hard for a user to adapt the code for their own needs. I believe that publishing some MATLAB notebooks commenting on the steps would be very useful.
Answer 2. Thank you for your comment. The author will add more documentation, like MATLAB notebooks, to improve the code adaptability and usability.
Comment 3: Similarly, tests are quite sparse. At the moment the tests only check that CasADI runs and that one particular example works, but these need to be ran manually. I would recommend some continuous integration on Github (or similar) to test things work periodically, but I don't know if that is feasible with proprietary software like MATLAB. In any case, some additional tests should be written, at the very least one to check that the parameter fitting tools work as expected.
Answer 3: Thank you for this comment. The author will add another test to ensure that the parameter optimization tool works properly.
Comment 4: At the moment the list of CONTRIBUTORS is a bit opaque. All the commits have been pushed by COBRAPROsimulator, which does not tell who that is or if is one or multiple users. This could be an issue if other people join the project or the person behind COBRAPROsimulator changes institutions. I would recommend having a list of contributors in the README, where contributors are acknowledged. The all-contributors tool is quite useful for this.
Answer 4: Thank you for this comment. We will list the contributors in the README file.
Minor comments:
Comment 5: The following comments refer to the installation instructions.
Comment 5.1:The instructions require SUNDIALS 2.6.2 but the hyperlink points to 2.6.1. In addition, this version is quite old. Is there any particular reason to not use a more recent version?
Answer 5.1: Thank you for pointing this out. The authors will fix the hyperlink so that SUNDIALS 2.6.2 is downloaded. In our accepted JES paper (hopefully published soon), we explain in detail why the old version of SUNDIALS is used. The robust single-step approach is used to determine consistent initial conditions [M. T. Lawder, V. Ramadesigan, B. Suthar, and V. R. Subramanian, “Extending explicit and linearly implicit ODE solvers for index-1 DAEs,” Computers & Chemical Engineering, vol. 82, pp. 283–292, Nov. 2015, doi: 10.1016/j.compchemeng.2015.07.002.] in COBRAPRO. This method request the use of implicit ODE solver, which the Casadi version of SUNDIALS (what pybamm seems to use) cannot support. Casadi version of SUNDIALS IDA solver only allows semi-explicit ODE or DAE solvers, while the SUNDIALS 2.6.2 version allows for implicit ODE/DAE solvers.
Comment 5.2: The instructions say CasADI should be installed, but they don't specify for which platform. Even though it is quite obvious, it would be helpful to specify it needs to be the MATLAB version.
Answer 5.2: Yes sure, this can be added to the README file.
Comment 5.3: The instructions assume MATLAB is installed, with some toolboxes. It should be stated explicitly at the beginning of the "Installation" section.
Answer 5.3: The authors will edit the README file such that it explicitly says that MATLAB and relevant toolboxes should be installed sat the beginning of the “Installation” section.
Comment 6: The following comments refer to the installation process in MATLAB:
Comment 6.1: There is a step that asks "Install toolbox" that it is not reflected in the installation instructions.
Answer 6.1: Thank you for your comment. This part will be added to the README file..
Comment 6.2: It would be good to always have a default answer to most of the prompts (so just hitting enter gets you through), as the fact that some answers require a yes and some a no is a bit confusing. If that is already implemented, users should be made aware of what the default is.
Answer 6.2: The authors will try to modify the installation file such that yes/no answers can be averted.
Comment 7: The title of the correlation matrix plot (e.g. when running DFN_LSA_Corr_CC.m) does not render LaTeX.
Answer 7: Thank you for catching that. We will fix that error.
Comment 8: The DFN_pso_0_05C.m also took a long time to me (around 30 minutes) and some errors/warning came up. It would be good to clarify in the documentation that this example might take a while to run.
Answer 8: Thank you for your comment. The authors will make it clear that the parameter optimization codes take a while to run. The errors/warnings that are printed to the screen are normal, as they notify the user why the simulation failed to work for the particular set of parameters in a PSO iteration.
Comment 9: The following comments refer to the article:
Comment 9.1: [l33] I would remove all the references here, as they are all cited again and discussed in more detail later on.
Answer 9.1: The authors will remove the reference here.
Comment 9.2: [l47] "Other packages resort to literature-derived parameters and lack the ability to predict battery behaviour". I think this statement is a bit of a stretch. All the aforementioned tools will provide models with the ability to predict real battery behaviour, as long as the user provides the right parameter values. COBRAPRO provides tools to estimate those parameters (which is no small feat), while the others don't, so I think this would be a more accurate statement.
Answer 9.2: Thank you for pointing this out. The authors will modify the paper so that we highlight that COBRAPRO is a closed-loop code that allows for parameter identification, whereas other codes are open-loop which already require correct parameters.
Comment 9.3: [l54]: "COBRAPRO leverages a fast solver". Which one? Give details and references.
Answer 9.3: The authors will clarify that COBRAPRO uses the SUNDIALS IDA solver. We will also reference our JES paper which provides more information on the SUNDIALS solver.
Comment 9.4: [l64] About Challenge 2, I think it is a bit misleading. I suspect most of the other tools also look for consistent initial conditions (PyBaMM definitely does, and I suspect COMSOL does too; haven't played much with the other tools). If any of the other tools does not do that, it should be stated clearly. Otherwise, I think this challenge would be better merged with Challenge 1, as it all ties to dealing with the computational complexity.
Answer 9.4: Unlike other DFN software, COBRAPRO implements the robust single-step approach [Lawder et al., 2015] which perturbs the algebraic variables into ODEs to prevent the use of nonlinear algebraic solvers and shown to have convergence issues. We have conducted a comparison study of the single-step method and the SUNDIALS IDACalc function, which uses a nonlinear algebraic solver, and showed that the single-step method implemented in COBRAPRO is more robust. This framework was first implemented in Maple in [Lawder et al., 2015] and implemented in MATLAB by DEARLIBS. We will provide more details on this in the JOSS paper.
Comment 9.5: [l185, but there are other occurrences through the text] The subscripts denoting labels (e.g. n, p, e...) should be in Roman text (i.e. \mathrm{e}) in LaTeX.
Answer 9.5.: Thank you for this comment. We will fix the paper accordingly.
Comment 9.6: [Fig3&5] The plots showing the SOC are not very insightful. I imagine they are computed through Coulomb counting in both cases, and given that the experiment is current driven they need to be virtually identical. I believe a plot of the voltage error between experiment and simulation would be a more valuable output of the code.
Answer 9.6: Thank you for this comment. COBRAPRO leverages the multi-objective cost function proposed by [A. Allam and S. Onori, “Online Capacity Estimation for Lithium-Ion Battery Cells via an Electrochemical Model-Based Adaptive Interconnected Observer,” IEEE Trans. Contr. Syst. Technol., vol. 29, no. 4, pp. 1636–1651, Jul. 2021, doi: 10.1109/TCST.2020.3017566.], which showed increased sensitivity of parameters by including SOCp and SOCn in the voltage cost function. This is why COBRAPRO outputs the voltage and also the SOC of the electrodes. This is also detailed in our JES paper, which we will include in COBRAPRO's README file.
Comment 9.7: [l246] Given that this list is bound to change as the code evolves, I think it would be better to not include it in the article and, instead, point the users to the README where an up-to-date list will be available.
Answer 9.7: Thank you for this valuable comment. The authors will remove this from the paper.
@BradyPlanden, thank you so much for your review. Here are our answers to your comments:
Repository Comments
Comment 1: While the authors provide an initial testing framework, this only covers conventional integration tests and lacks individual unit tests or providing users with a coverage metric.
Answer 1: Thank you for this comment. Could the reviewer please explain more in detail what additional testing codes should be developed to improve the code? Currently, the authors included a test to ensure that the DFN model is simulated correctly by including a comparison with COMSOL results. If this test is successful, it implies that all the sub function files are working well as well. Following @brosaplanella comment, the author will include an additional test to ensure that the parameter optimization is working properly.
Comment 2: In its current state, the repository appears to be more of a collection of scripts rather than a package/module/etc. This makes it difficult to determine the software architecture, potentially limiting the reach of this work. This repository provides a reference to a possible structure (as a MATLAB codebase): https://github.com/Battery-Intelligence-Lab/BatEst
Answer 2: Thank you for this comment. The author will improve the organization of COBRAPRO such that the software architecture is easier to comprehend. For example, the author can create subfolders in the DFN_model folder to provide more granular information on the code structure.
Comment 3: More functionality could be added to the particle swarm optimisation object (i.e. max iterations, time, etc.) with documentation. Specifically, here: https://github.com/COBRAPROsimulator/COBRAPRO/blob/157ab80ffda30aa7c51c39cc7a042162efbfbaf0/Examples/Parameter_Identification_Routines/DFN_pso_0_05C.m#L198
Answer 3: Thank you for this comment. The author will modify the code such that the PSO setting options can be input by the user.
Article Comments
Comment 4: Presently, the comparison with alternative work in the field is aimed at pure forward modelling packages (PyBaMM, PetLion.jl, LionSimba) which do not aim to solve the inverse problem, a more extensive literature review will find other implementations which are more suitable for comparison.
Answer 4: Thank you for this comment. The authors focused on comparing DFN open-source codes, since COBRAPRO is a DFN code software. The authors did not include codes using SPM, ESPM, or electric-circuit models in the comparison. The authors were not able to find other DFN open-source codes that include a closed-loop optimization framework other than DEARLIBS. If the reviewer is aware of other DFN codes with a parameter identification framework, please let us know. Thank you. Furthermore, a more detailed study comparing the different DFN codes is included in our accepted JES paper which we will cite in COBRAPRO's GitHub page.
Comment 5: A table showing the full set of parameters identified for the example would be helpful to summarise the results in one place for the reader.
Answer 5: Thank you for this comment. The author can indeed include a table showing the full list of DFN parameters. Or, we can cite our JES paper, which has a detailed list of the DFN parameters to save space in the JOSS paper.
Comment 6: Similarly, a table of correlation metrics for the parameters in the LSA section of the example.
Answer 6: Thank you for this comment. The authors can add more details on the LSA and correlation analysis results for improved clarity. Also, please note that our JES paper on COBRAPRO provides a detailed account of the LSA and correlation analysis methods.
@BradyPlanden and @brosaplanella, can you please have a look at the fixes and replies provided by @COBRAPROsimulator and see if they resolve the issues raised by you? If so, I appreciate it if you finalize your reviews.
Hi! Even though the comments have been replied to, there have been no changes in the article nor the repository (or at least I couldn't find them) so I can't approve them yet.
@brosaplanella Ah, I see. @COBRAPROsimulator can you please reflect on the above comment?
@mbarzegary and @brosaplanella, I am working on implementing the comments. I will let you know when they are complete. Thank you very much.
Hello, @mbarzegary, @brosaplanella, @BradyPlanden, thank you again for your valuable comments. We wanted to let you know what we are in the process of making those changes, in the code and the paper. When complete, we will respond with a detailed list of the changes we made to address your comments. Thank you.
@COBRAPROsimulator can you please provide an update of your progress on fixing the issues?
Hello @mbarzegary, I will provide updates to @brosaplanella and @BradyPlanden's comments shortly. Thank you for your patience.
Hello @brosaplanella, thank you again for your valuable suggestions. We have made changes to our code and paper accordingly. In addition to our responses to your comments in the thread above, please find the actions we took for each of your comments below. Please let us know any further clarification you require and other suggestions/comments you may have. Thank you very much.
Major comments
One of the main claims in the paper is performance, and it is stated that the 1C discharge runs in less than a second (similar order of magnitude to PyBaMM). When running it on my laptop (it tic/toced the first 1C discharge in cycle_CC.m it took around 8 seconds (the PyBaMM time in my machine is similar to that reported by the authors). I understand that the discrepancy is probably due to pre/post-processing and printing to screen, but it would be good to have one example with minimal overhead which can demonstrate the performance claimed in the article.
[x] Actions taken: In line 344 of the DFN_model/runModel.m function, the script will now print the time it took to solve the DAE to the Command Window. This way, if the printing option is enabled, users will be able to readily view the DAE solver time without having to resort to tic/toc. Furthermore, in Examples/Cycling/cycle_CC.m, lines 141-148 now prints the DAE solver time for each cycle and the user defined total number of cycles. For 1C discharge with Np=Ns=Nn=Nrp=Nrn=10, it can be verified with cycle_CC.m that it takes less than 1 second to run one cycle, as claimed in the paper. Also, please refer to our JES paper for a comprehensive computation time comparison study: S. Ha and S. Onori, “COBRAPRO: An Open-Source Software for the Doyle-Fuller-Newman Model with Co-Simulation Parameter Optimization Framework,” J. Electrochem. Soc., vol. 171, no. 9, p. 090522, Sep. 2024, doi: 10.1149/1945-7111/ad7292.
I agree with @yuefan98 that the documentation is a bit sparse. Even though for a research tool the current documentation might be enough, it should be expanded in the near future (I am also happy for this to take place after publication). I believe the main issue is that, even though the examples work straight out of the box, it would be extremely hard for a user to adapt the code for their own needs. I believe that publishing some MATLAB notebooks commenting on the steps would be very useful.
[x] Actions taken: A readthedocs have been created for COBRAPRO. This includes the API documentation listing the functions in COBRAPRO. The authors plan to expand upon readthedocs to include examples/tutorials to allow for a more intuitive user experience. The readthedocs is still a work in progress and it is now linked on the README page.
Similarly, tests are quite sparse. At the moment the tests only check that CasADI runs and that one particular example works, but these need to be ran manually. I would recommend some continuous integration on Github (or similar) to test things work periodically, but I don't know if that is feasible with proprietary software like MATLAB. In any case, some additional tests should be written, at the very least one to check that the parameter fitting tools work as expected.
[x] Actions taken: We have included an additional test, test/test3_psoCheck.m, that checks that the particle swarm optimization function is working in the parallel structure, as required by the parameter optimization toolbox in COBRAPRO.
At the moment the list of CONTRIBUTORS is a bit opaque. All the commits have been pushed by COBRAPROsimulator, which does not tell who that is or if is one or multiple users. This could be an issue if other people join the project or the person behind COBRAPROsimulator changes institutions. I would recommend having a list of contributors in the README, where contributors are acknowledged. The all-contributors tool is quite useful for this.
[ ] Actions taken: The authors will use the all-contributors tool to list all the contributors in the README page. We are still in the process of setting this up, but wanted to inform you that we have taken action on your advice.
Minor comments
The instructions require SUNDIALS 2.6.2 but the hyperlink points to 2.6.1. In addition, this version is quite old. Is there any particular reason to not use a more recent version?
[x] Actions taken: The link has been fixed to SUNDIALS 2.6.2. To add to my response above on why the older version of SUNDIALS is used, please refer to our JES paper that explains this further: S. Ha and S. Onori, “COBRAPRO: An Open-Source Software for the Doyle-Fuller-Newman Model with Co-Simulation Parameter Optimization Framework,” J. Electrochem. Soc., vol. 171, no. 9, p. 090522, Sep. 2024, doi: 10.1149/1945-7111/ad7292.
The instructions say CasADI should be installed, but they don't specify for which platform. Even though it is quite obvious, it would be helpful to specify it needs to be the MATLAB version.
[x] Actions taken: In the installation directions of the README file, it has been specified that the MATLAB version of CasADi needs to be installed.
The instructions assume MATLAB is installed, with some toolboxes. It should be stated explicitly at the beginning of the "Installation" section.
[x] Actions taken: The installation directions of the README file has been edited such that the first step is to download MATLAB and the necessary MATLAB toolboxes.
There is a step that asks "Install toolbox" that it is not reflected in the installation instructions.
[x] Actions taken: The “Install toolbox” step has been added to the installation directions in the README file.
It would be good to always have a default answer to most of the prompts (so just hitting enter gets you through), as the fact that some answers require a yes and some a no is a bit confusing. If that is already implemented, users should be made aware of what the default is.
[x] Actions taken: Since the yes/no answers are embedded as part of the SUNDIALS installation function, it seems that there is no way to edit or change the answers. Furthermore, the yes/no answers allow users to download specific parts of the SUNDIALS package, which provides users with more flexibility.
The title of the correlation matrix plot (e.g. when running DFN_LSA_Corr_CC.m) does not render LaTeX.
[x] Actions taken: It is now verified that all LaTeX renderings in DFN_LSA_Corr_CC.m work as expected.
The DFN_pso_0_05C.m also took a long time to me (around 30 minutes) and some errors/warning came up. It would be good to clarify in the documentation that this example might take a while to run.
[x] Actions taken: It has been noted in the README file that the parameter identification example files take a while to run. Also, the files parameter_identification/pso_objectiveFunctions/DFN_obj_CC.m and parameter_identification/pso_objectiveFunctions/DFN_obj_HPPC.m have been edited such that the messages printed to the Command Window are clearer to understand. The message now prints what occurred in the objective function, such as ‘'Model failed to initialize algebraic variables for given PSO parameter set” or “'PSO caught unfeasible solution” rather than printing “errors” which can be confusing to the users, as these are not errors but rather instances of the DFN model unable to be solved given the parameter guesses by PSO.
[l33] I would remove all the references here, as they are all cited again and discussed in more detail later on.
[x] Actions taken: The reference has been removed from [l33].
[l47] "Other packages resort to literature-derived parameters and lack the ability to predict battery behaviour". I think this statement is a bit of a stretch. All the aforementioned tools will provide models with the ability to predict real battery behaviour, as long as the user provides the right parameter values. COBRAPRO provides tools to estimate those parameters (which is no small feat), while the others don't, so I think this would be a more accurate statement.
[x] Actions taken: This part of the paper has been modified to clarify that COBRPRO allows the determination of parameters (closed-loop software), whereas other open-source DFN codes require pre-determined parameters (open-loop software) to simulate real-world batteries.
[l54]: "COBRAPRO leverages a fast solver". Which one? Give details and references.
[x] Actions taken: The paper has been edited to specify that the SUNDIALS IDA solver is used and relevant references have been added as well.
[l64] About Challenge 2, I think it is a bit misleading. I suspect most of the other tools also look for consistent initial conditions (PyBaMM definitely does, and I suspect COMSOL does too; haven't played much with the other tools). If any of the other tools does not do that, it should be stated clearly. Otherwise, I think this challenge would be better merged with Challenge 1, as it all ties to dealing with the computational complexity.
[x] Actions taken: Challenge 2 has been rewritten to highlight and distinguish the consistent initial condition method used in COBRAPRO and other open-source DFN codes. COBRAPRO uses the single-step approach, which perturbs algebraic equations into implicit ordinary differential equations, preventing the need for a nonlinear algebraic solver like SUNDIALS IDACalcIC or CasADi rootfinder. Nonlinear algebraic solver are typically used in other open-source DFN codes as highlighted in our JES paper, which has been shown to be less robust than the single-step method at determining consistent initial conditions at various discretization points and C-rates.
[l185, but there are other occurrences through the text] The subscripts denoting labels (e.g. n, p, e...) should be in Roman text (i.e. \mathrm{e}) in LaTeX.
[x] Actions taken: Authors note that the variables in [l185] (now [l158] in the new version) refer to the variable names in the code, not the LaTeX versions of the parameter names.
[Fig3&5] The plots showing the SOC are not very insightful. I imagine they are computed through Coulomb counting in both cases, and given that the experiment is current driven they need to be virtually identical. I believe a plot of the voltage error between experiment and simulation would be a more valuable output of the code.
[x] Actions taken: In [l93] of the new version, authors have added more details and cited the multi-objective function proposed by [A. Allam and S. Onori, “Online Capacity Estimation for Lithium-Ion Battery Cells via an Electrochemical Model-Based Adaptive Interconnected Observer,” IEEE Trans. Contr. Syst. Technol., vol. 29, no. 4, pp. 1636–1651, Jul. 2021, doi: 10.1109/TCST.2020.3017566]. This highlights the reason for including the SOC plots in the identification results.
[l246] Given that this list is bound to change as the code evolves, I think it would be better to not include it in the article and, instead, point the users to the README where an up-to-date list will be available.
[x] Actions taken: The example files and location have been removed from the paper. The authors plan to include update the readthedocs accordingly as more examples are included in the future.
@editorialbot generate pdf
:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
Hi! The changes look good to me, happy for the paper to be published.
Hello @BradyPlanden, thank you again for your helpful comments. We have made changes to our code and paper to reflect your comments. Below, please find the action items we took for each of your comments. Please let us know if you require clarification on any items discussed.
Repository Comments
While the authors provide an initial testing framework, this only covers conventional integration tests and lacks individual unit tests or providing users with a coverage metric.
[x] Actions taken: We have included an additional test (test/test3_psoCheck.m) that checks for correct implementation of the particle swarm optimization function in parallel structure, as required by the parameter optimization toolbox in COBRAPRO. By individual unit tests, does this mean providing a way to check that each MATLAB function works correctly? The authors would greatly appreciate additional details/information to improve the testing metrics of COBRAPRO.
In it's current state, the repository appears to be more of a collection of scripts rather than a package/module/etc. This makes it difficult to determine the software architecture, potentially limiting the reach of this work. This repository provides a reference to a possible structure (as a MATLAB codebase): https://github.com/Battery-Intelligence-Lab/BatEst
[x] Actions taken: The authors have looked at the BatEst repository and categorized COBRAPRO in a similar manner such that each folder and subfolders are more intuitively organized. For instance, the original COBRAPRO folders were organized as “DFN_identification” (included PSO related functions), “DFN_identification_results” (included identified results in .mat files), “DFN_model” (included all DFN model related functions), “DFN_parameters” (included DFN parameter files), “Local_Sensitivity Analysis”, “Examples”, and “test”. Now, the folders are organized as “data” (includes battery data used in our JES paper), “parameters” folder (consisting of subfolders for OCP and electrolyte-related functions), “parameter_identification” (consisting of subfolders “identifiability_analysis” and “pso_objectiveFunctions”, and “DFN_model” (consisting of subfolders “additional equations”, “DAE”, FVM_interpolation”, and “helper_functions”). The “parameters” folder is organized in this manner such that the functional parameters (OCP and electrolyte-related parameters) can be saved in their respective folders. The “parameter_identification” folder includes the closed-loop PSO function and parameter identifiability analysis (LSA and correlation analysis), rather than having them in separate main folders. The “DFN_model” subfolders are organized according to the code’s functions, such as “DAE” for function related to setting up the DAE, “FVM_interpolation” for numerical discretization related functions, “additional_equations” for functions to calculate the particle concentration related equations once the DAE is solved, and “helper_functions” for accessory functions to aid the main function, runModel.m.
More functionality could be added to the particle swarm optimisation object (i.e. max iterations, time, etc.) with documentation. Specifically, here: https://github.com/COBRAPROsimulator/COBRAPRO/blob/157ab80ffda30aa7c51c39cc7a042162efbfbaf0/Examples/Parameter_Identification_Routines/DFN_pso_0_05C.m#L198
[x] Actions taken: The files Examples/Parameter_Identification_Routines/DFN_pso_0_05C.m and Examples/Parameter_Identification_Routines/DFN_pso_HPPC.m have been edited to allow users to modify the PSO settings. For DFN_pso_0_05C.m, lines 83-87 can be edited to change the particle number, lines 89-106 allow users to determine the exit conditions in the PSO, and lines 108-116 can be edited to change the weight settings in PSO (appropriate references have been added for users to learn more about PSO weight settings). Similarly, DFN_pso_HPPC.m has been edited to include the same new features in lines 90-123.
Article Comments
Presently, the comparison with alternative work in the field is aimed at pure forward modelling packages (PyBaMM, PetLion.jl, LionSimba) which do not aim to solve the inverse problem, a more extensive literature review will find other implementations which are more suitable for comparison.
Actions taken: In lines 38-46 of the paper, authors have specified that the comparison is made between open-source codes for the DFN model. While other open-source codes may be available for the SPM, ESPM, and ECM that include parameter identification routines, we highlight that there is a lack of parameter identification codes for the DFN model other than DEARLIBS. The DFN model is the most computationally complex and highest fidelity model compared to the reduced-order forms and the empirical ECM. For this reason, the DFN model provides unique advantages for material design and advanced BMS applications, highlighting the need for an open-source DFN code that can determine accurate parameters through parameter identification. To further clarify, we do not solve the inverse problem (we do not inverse the dynamic equations), but rather we are using an iterative optimization process to determine parameters using battery voltage and current data. Please let us know if we can clarify any other points.
A table showing the full set of parameters identified for the example would be helpful to summarise the results in one place for the reader.
[x] Actions taken: To adhere to JOSS’s paper length, authors have included in line 133 a reference to our JES paper, which includes a comprehensive table listing all the DFN model parameters.
Similarly, a table of correlation metrics for the parameters in the LSA section of the example.
[x] Actions taken: Authors have rewritten the LSA section in the example to clarify how the identifiability analysis works. Now, this part is titled “Parameter Identifiability Analysis” and shows how to determine an identifiable parameter set using LSA and correlation analysis. The user defines a correlation threshold, which is then used to determine the if any of two parameters are correlated or not. The algorithm behind this is explained in detail in our JES paper, S. Ha and S. Onori, “COBRAPRO: An Open-Source Software for the Doyle-Fuller-Newman Model with Co-Simulation Parameter Optimization Framework,” J. Electrochem. Soc., vol. 171, no. 9, p. 090522, Sep. 2024, doi: 10.1149/1945-7111/ad7292, which is cited in this section. The correlation threshold is the only user defined metric for this example, as described in the new version of the paper in lines 204-208.
@BradyPlanden, can you please have a look at the above replies provided by @COBRAPROsimulator and see if they resolve your concerns? I see that the Performance item is not checked in your review list yet.
Yes - thanks for the update @COBRAPROsimulator. Happy for this to proceed @mbarzegary.
Perfect. Thank you for finalizing your reviews @brosaplanella and @BradyPlanden.
@editorialbot generate pdf
:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
@editorialbot check references
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):
✅ OK DOIs
- 10.1149/1.2221597 is OK
- 10.1149/2.0301603jes is OK
- 10.1016/j.energy.2022.125966 is OK
- 10.1149/1945-7111/ab7bd7 is OK
- 10.1149/2.0551509jes is OK
- 10.1149/2.0321816jes is OK
- 10.1149/1945-7111/ab9050 is OK
- 10.5334/jors.309 is OK
- 10.1149/2.0291607jes is OK
- 10.1149/2.0171711jes is OK
- 10.1016/j.jpowsour.2016.12.083 is OK
- 10.4271/2023-01-5047 is OK
- 10.1016/j.dib.2022.107995 is OK
- 10.1149/1945-7111/ac201c is OK
- 10.1149/1945-7111/ad7292 is OK
- 10.1149/2.0051908jes is OK
- 10.1149/1945-7111/ac22c8 is OK
- 10.1016/j.compchemeng.2011.01.003 is OK
- 10.1016/j.compchemeng.2015.07.002 is OK
- 10.1149/1945-7111/ad1293 is OK
- 10.1145/3539801 is OK
- 10.1145/1089014.1089020 is OK
- 10.1016/j.ensm.2021.10.023 is OK
- 10.1149/1945-7111/ad1293 is OK
- 10.1109/TCST.2020.3017566 is OK
- 10.1149/1945-7111/ab7091 is OK
- 10.1016/j.jpowsour.2012.03.009 is OK
🟡 SKIP DOIs
- No DOI given, and none found for title: COMSOL Multiphysics© v. 6.2
- No DOI given, and none found for title: Battery Design Module User’s Guide, COMSOL Multiph...
- No DOI given, and none found for title: fastDFN
❌ MISSING DOIs
- None
❌ INVALID DOIs
- None
@editorialbot set <DOI here> as archive
@editorialbot set <version here> as version
@editorialbot generate pdf
@editorialbot check references
and ask author(s) to update as needed@editorialbot recommend-accept
@COBRAPROsimulator given the green light of the reviewers, we will now work towards processing this for acceptance in JOSS. So please
I can then move forward with recommending acceptance of the submission.
Hi @mbarzegary, thank you very much for initiating the post-review process. I have completed the post-review tasks as you mentioned:
Finally, I have merged your PR containing the minor edits. Thank you for pointing out the spelling errors.
Submitting author: !--author-handle-->@COBRAPROsimulator<!--end-author-handle-- (Sara Ha) Repository: https://github.com/COBRAPROsimulator/COBRAPRO Branch with paper.md (empty if default branch): Version: v1.0.0 Editor: !--editor-->@mbarzegary<!--end-editor-- Reviewers: @yuefan98, @BradyPlanden, @brosaplanella Archive: Pending
Status
Status badge code:
Reviewers and authors:
Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)
Reviewer instructions & questions
@yuefan98 & @BradyPlanden & @brosaplanella, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @mbarzegary know.
✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨
Checklists
📝 Checklist for @yuefan98
📝 Checklist for @brosaplanella
📝 Checklist for @BradyPlanden