[GSOC 2025] Saving and Loading Parameter Sets #5134
Conversation
|
Hi @medha-14, I left a note on your other PR about how you can help us access a better diff: #5137 (comment) cc: @Saransh-cpp @santacodes @valentinsulzer for reviews |
There was a problem hiding this comment.
@medha-14 left a note for you to work on, I will review it once again after my suggestion is addressed.
| "my_model.json", battery_model=pybamm.lithium_ion.BaseModel() | ||
| ) | ||
|
|
||
| # ✅ Process the model AFTER update() so the necessary .input_* keys get created |
There was a problem hiding this comment.
@medha-14 Could you clean up the comments and refine the test functions more, seems like this was generated through an LLM, not against it, I just want to be sure that we keep the code readable and works as intended if you haven't tested it. I think you can add a short docstring above each test function instead of comments. Also, if you see any test functions that are irrelevant or not needed, feel free to remove them.
There was a problem hiding this comment.
I have the same suggestion here. I am also not against the use of AI models to generate code, but you must understand what the generated code does both from a microscopic level (say, what's happening in a specific test) and from a macroscopic one (what feature is being implemented, does the API match what we need, are we reusing functions we've previously defined, whether or not the AI system being used is confabulating code or not, etc.).
3 participants
Description
This PR is based on #5056 and contains changes that build upon it.
Once #5056 is merged, the diff for this PR will automatically update to show only the relevant changes.
Type of change
Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #)
Important checks:
Please confirm the following before marking the PR as ready for review:
nox -s pre-commitnox -s testsnox -s doctests