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

Feature/battery cell model #91

Merged
merged 16 commits into from
Mar 28, 2024

Conversation

ZackTully
Copy link
Collaborator

This is a more detailed battery model called LIB based on the lithium ion cell model in [1.]. The LIB model calculates charging/discharging losses due to internal resistance and transient behavior.

This PR also includes initial documentation for the battery model #68.

References:

  1. M.-K. Tran et al., “A comprehensive equivalent circuit model for lithium-ion batteries, incorporating the effects of state of health, state of charge, and temperature on model parameters,” Journal of Energy Storage, vol. 43, p. 103252, Nov. 2021, doi: 10.1016/j.est.2021.103252.

@ZackTully ZackTully added documentation Improvements or additions to documentation new-feature A new feature labels Mar 25, 2024
Copy link
Collaborator

@misi9170 misi9170 left a comment

Choose a reason for hiding this comment

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

This looks fine to me aside from a few small comments below.

I did run into a bit of trouble when running the example though---I found that, in order to run the example, the file outputs/log_test_client.log had to already exist in the example. I checked, and the same was true for the 02 example, so I don't think it's this PR that's causing the problem. @genevievestarke , have you run into this before? Can one of you confirm that if you delete the outputs/log_test_client.log file, the example does not run for you? If so, I'll open an issue.

EDIT: @genevievestarke and I tracked this issue with outputs/log_test_client.log down, and there is now a bugfix for it in #93

tests/python_simulators_test/battery_test.py Outdated Show resolved Hide resolved
example_case_folders/02_amr_wind_standin_only/loghelics Outdated Show resolved Hide resolved
tests/python_simulators_test/battery_test.py Outdated Show resolved Hide resolved
hercules/emulator.py Outdated Show resolved Hide resolved
@ZackTully
Copy link
Collaborator Author

@misi9170 I tried running both example 02 and 06 and they seem to work mostly fine for me.

I tried:

  1. example 2 with whole outputs folder deleted.
  2. example 2 with only log_test_client.log deleted.
  3. example 6 with outputs folder deleted.
  4. example 6 with only log_test_client.log deleted.

Case 1 was the only one that was unusual. When I ran the bash script, it would create the outputs folder then get stuck and not run the simulation. If I ran the bash script a second time, the outputs folder would be populated with the appropriate files.

@misi9170
Copy link
Collaborator

@misi9170 I tried running both example 02 and 06 and they seem to work mostly fine for me.

I tried:

1. example 2 with whole `outputs` folder deleted.

2. example 2 with only `log_test_client.log` deleted.

3. example 6 with `outputs` folder deleted.

4. example 6 with only `log_test_client.log` deleted.

Case 1 was the only one that was unusual. When I ran the bash script, it would create the outputs folder then get stuck and not run the simulation. If I ran the bash script a second time, the outputs folder would be populated with the appropriate files.

Thanks @ZackTully ! That's consistent with what I was getting, exact that for me, it seems that I was getting the same problem you describe for Case 1 when trying Case 3. Anyway, I think that #93 should fix this; we'll hopefully get that one in soon and that should clear the way for this PR to go in

@misi9170 misi9170 self-requested a review March 26, 2024 20:33
Copy link
Collaborator

@misi9170 misi9170 left a comment

Choose a reason for hiding this comment

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

Approving now, but I think we should wait until #93 goes in before merging

@genevievestarke
Copy link
Collaborator

Which bash script are we using for this? The file 'batch_script.sh' seems most complete, but I'm getting odd output from the amr wind standin data part that we might want to look into? It looks like the wind speed in amr_standin_data.csv is just half the time value.

@genevievestarke
Copy link
Collaborator

Nvm, I figured it out, all good!

@genevievestarke
Copy link
Collaborator

It doesn't seem like the battery is being used in this example, or the power outputs aren't being updated. I thought we had some basic signals passed from WHOC in this example? Is this something we need to work out about the communication between the two (Hercules and WHOC)?

@ZackTully
Copy link
Collaborator Author

It doesn't seem like the battery is being used in this example, or the power outputs aren't being updated. I thought we had some basic signals passed from WHOC in this example? Is this something we need to work out about the communication between the two (Hercules and WHOC)?

Thanks for catching that! It's a result of updating WHOC/HERCULES communication when the emulator main dictionary wasn't writing to the output file. The changes to WHOC are in this pull request: NREL/wind-hybrid-open-controller#33

I think that should solve the problem.

Copy link
Collaborator

@genevievestarke genevievestarke left a comment

Choose a reason for hiding this comment

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

Ok, sounds good! I think we can merge this in!

@genevievestarke genevievestarke merged commit 070423f into NREL:develop Mar 28, 2024
6 checks passed
@misi9170 misi9170 mentioned this pull request Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation new-feature A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants