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

Experiment Drive Cycle with Events #1304

Merged
merged 14 commits into from
Mar 29, 2021
Merged

Conversation

alibh95
Copy link
Contributor

@alibh95 alibh95 commented Dec 23, 2020

Description

Added the functionality of simulating drive cycles in Experiment class along with the 'for' event.
If the time specified in the 'for' event is less than the drive cycle time, then the drive cycle is stopped at the time specified in the 'for' event.
Else If the time specified in the 'for' event is greater than the drive cycle time, the drive cycle first extended for one week and then stopped at the time specified in the 'for' event.

Fixes # (issue)

#1193
#1293 (Can extend single and/or multiple drive cycles for long time durations)

Note:

I am new to GitHub. I was not able to manage my previous pull request (#1279) effectively. Therefore, I am opening this new pull request. I am sorry for any inconvenience I caused.

Warning

In this PR, Only the Experiment class is modified to support drive cycle functionality. This functionality is not actually implemented in Simulation yet.

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.

  • New feature (non-breaking change which adds functionality)
  • Optimization (back-end change that speeds up the code)
  • Bug fix (non-breaking change which fixes an issue)

Key checklist:

  • No style issues: $ flake8
  • All tests pass: $ python run-tests.py --unit
  • The documentation builds: $ cd docs and then $ make clean; make html

You can run all three at once, using $ python run-tests.py --quick.

Further checks:

  • Code is commented, particularly in hard-to-understand areas
  • Tests added that prove fix is effective or that feature works

Added the functionality of simulating drive cycles in Experiment class along with 'for' Event.
If the time specified in 'for event is less than the drive cycle time, the drive cycle is stoped at the time specified in 'for' event.
Else If the time specified in 'for event is greater than the drive cycle time, the drive cycle first extended for 1 week and then stoped at the time specified in 'for' event.
@codecov
Copy link

codecov bot commented Dec 23, 2020

Codecov Report

Merging #1304 (dab2451) into develop (daf7fa7) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1304      +/-   ##
===========================================
- Coverage    98.28%   98.27%   -0.01%     
===========================================
  Files          280      280              
  Lines        16165    16198      +33     
===========================================
+ Hits         15887    15918      +31     
- Misses         278      280       +2     
Impacted Files Coverage Δ
pybamm/experiments/experiment.py 97.40% <100.00%> (-0.95%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update daf7fa7...dab2451. Read the comment docs.

@alibh95 alibh95 changed the title Experiment Drive Cycle with 'for' Event Experiment Drive Cycle with Events Dec 24, 2020
@brosaplanella
Copy link
Member

Hi @alibh95! The errors in the MacOs tests should now be fixed if you merge in the develop branch. Let me know if the PR is ready to be reviewed.

@brosaplanella
Copy link
Member

Hi @alibh95! What's the state of this PR?

@alibh95
Copy link
Contributor Author

alibh95 commented Feb 3, 2021

Hi @brosaplanella . This pull request successfully simulates drive cycles using experiment class. The implementation of for event have been completed. The drive cycle is clipped or extended depending upon the time period provided in the for event.
Unfortunately, I am unable to spare much time for the successful implementation of until event because I am heavily occupied in completing my thesis.
The issue I am facing in the implementation of until event is that whenever until event is triggered, the solver continues to solve the drive cycle instead of stoping.
Any tips/suggestions would be helpful for me to complete this PR.

@valentinsulzer
Copy link
Member

Hi @alibh95 , thanks for taking this on. We're going to implement this slightly differently in the simulation now that #1408 allows a better approach. Still, what you have added to the Experiment class is useful - could you change the PR to include only those changes (not the changes to the Simulation class)? Also add a warning that the drive cycle functionality is not actually implemented in Simulation yet.

examples/scripts/experiment_drive_cycle.py Outdated Show resolved Hide resolved
pybamm/experiments/experiment.py Outdated Show resolved Hide resolved
pybamm/experiments/experiment.py Outdated Show resolved Hide resolved
Copy link
Member

@valentinsulzer valentinsulzer left a comment

Choose a reason for hiding this comment

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

Thanks @alibh95 , looks good now. We're about to do a release, I will wait until that is done before merging as otherwise it might be confusing why you can specify a drive cycle in the experiment but not solve it

@valentinsulzer
Copy link
Member

@all-contributors add @alibh95 for code, tests

@allcontributors
Copy link
Contributor

@tinosulzer

I've put up a pull request to add @alibh95! 🎉

@valentinsulzer valentinsulzer merged commit b020a6c into pybamm-team:develop Mar 29, 2021
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.

3 participants