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

Potential 'output_writers' saving bug? #3614

Closed
mncrowe opened this issue Jun 5, 2024 · 6 comments
Closed

Potential 'output_writers' saving bug? #3614

mncrowe opened this issue Jun 5, 2024 · 6 comments

Comments

@mncrowe
Copy link

mncrowe commented Jun 5, 2024

I've run a simulation with a timestep of $\Delta t = 1.066666...$ and it looks as though there's some kind of rounding bug which is resulting in multiple saves occasionally. I choose 'schedule = TimeInterval(t_end/num_saves)' and expect to get 'num_saves' (or possibly 'num_saves+1') points in the saved 'time' variable. However, I end up with about 30 extra saves. Examining 'time' in more detail reveals that most of the points are separated by 't_end/num_saves' but a few (around 30) are separated by a much smaller interval. My guess is that this is a rounding bug with when the code saves the field. Alternatively I could have messed something up.

Matt

@glwagner
Copy link
Member

glwagner commented Jun 5, 2024

I've seen this before and yes I think it's a bug. Is there another issue open about this? I think there may be. @tomchor has a PR that is looking at this, we could use some extra eyes though.

@tomchor
Copy link
Collaborator

tomchor commented Jun 5, 2024

I also reported this on #3056, so maybe we should close this to avoid multiple issues of the same problem.

I don't have a PR specifically trying to fix that, but it is possible that #3606 fixes, or least ameliorates, the issue.

@mncrowe can you test your simulation on that branch and check if you see the same behavior?

@tomchor tomchor closed this as completed Jun 5, 2024
@glwagner
Copy link
Member

glwagner commented Jun 5, 2024

@mncrowe it sounds like this happened when you weren't using adaptive time-stepping --- which is nice because it will help us construct an MWE more easily. Do you think you can help with that? We just need a very simple simulation that reproduces the issue, hopefully something 0D with no dynamics that runs fast.

@mncrowe
Copy link
Author

mncrowe commented Jun 5, 2024

@glwagner Will do. I'll remove the dynamics from my run and see if I can get a minimum not-working example.

@tomchor I've downloaded that branch and will test when I can.

Our system seems to have forgotten it's got GPUs post power cut so it's CPU only for now, I assume the underlying save functions are the same between devices?

@tomchor
Copy link
Collaborator

tomchor commented Jun 5, 2024

Our system seems to have forgotten it's got GPUs post power cut so it's CPU only for now, I assume the underlying save functions are the same between devices?

Yes. CPUs are even better for MWEs since we can run them from any laptop.

Could you please post the MWE on #3056? That way we centralize the discussion there.

@glwagner
Copy link
Member

glwagner commented Jun 6, 2024

I don't think the MWE even requires saving output. Can't we also achieve an MWE using a simple Callback?

This issue should be independent of whether you are using the CPU or GPU.

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

No branches or pull requests

3 participants