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

Fix #128: save_every condition in ExportCallBack #125

Merged

Conversation

mstoelzle
Copy link
Contributor

The previous implementation would require "skip_step" and "save_every" to be synchronous in order to trigger. Now, we instead count the number of time steps since the last save and trigger the save whenever this number is higher or equal than the "save_every" parameter

skim0119 and others added 2 commits June 17, 2022 09:24
The previous implementation would require "skip_step" and "save_every" to be synchronous in order to trigger. Now, we instead count the number of time steps since the last save and trigger the save whenever this number is higher or equal than the "save_every" parameter
@skim0119
Copy link
Collaborator

skim0119 commented Jul 4, 2022

@mstoelzle Hi, Thank you for your interest in PyElastica.

Can you clarify the bug a little more, maybe specify the expected behavior?

Maybe I should have named and documented the variables more clearly; skip_step and save_every has two different purposes. The parameter skip_step is the number of step intervals to save the simulation. For example, if dt=1e-4 and skip_step=1000, the data will save with the interval of 0.1 second. These data will be saved in the buffer dictionary.

On the other hand, save_every specify the number of timesteps before this dictionary is dumped into the file. For example, if save_every=1e8, the save-file will be generated every 1000 seconds of simulation. The purpose is to reduce the number of files generated during the simulation.

If you have a different idea to implement, we would appreciate it if you make an issue to discuss further.

@mstoelzle mstoelzle changed the title Fix bug with save_every condition in ExportCallBack Fix #128: save_every condition in ExportCallBack Jul 4, 2022
…inimizing any potential delays for saving into storage
@skim0119 skim0119 changed the base branch from master to update-0.2.4 July 4, 2022 14:47
@skim0119 skim0119 self-requested a review July 4, 2022 14:55
@skim0119 skim0119 added this to the Version 0.2.4 milestone Jul 4, 2022
@skim0119 skim0119 added the bug Something isn't working label Jul 4, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #125 (c2b18f7) into update-0.2.4 (2022949) will not change coverage.
The diff coverage is 100.00%.

❗ Current head c2b18f7 differs from pull request most recent head a860370. Consider uploading reports for the commit a860370 to get more accurate results

@@              Coverage Diff              @@
##           update-0.2.4     #125   +/-   ##
=============================================
  Coverage         87.09%   87.09%           
=============================================
  Files                40       40           
  Lines              2534     2534           
  Branches            343      343           
=============================================
  Hits               2207     2207           
  Misses              305      305           
  Partials             22       22           
Impacted Files Coverage Δ
elastica/callback_functions.py 83.82% <100.00%> (ø)

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 2022949...a860370. Read the comment docs.

@skim0119 skim0119 merged commit 5b53617 into GazzolaLab:update-0.2.4 Jul 4, 2022
@skim0119
Copy link
Collaborator

skim0119 commented Jul 4, 2022

@mstoelzle Thanks. I'm squash-merging, so please re-clone downstream if there is any.

Regarding the path assertion, I'll disable the assertion for now (536a49e) and make an update with #127 later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug for asynchronous specs of skip_step and save_every in ExportCallBack
3 participants