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

Model implicitly assumes Clock.Δt is constant. #37

Closed
ali-ramadhan opened this issue Feb 10, 2019 · 5 comments
Closed

Model implicitly assumes Clock.Δt is constant. #37

ali-ramadhan opened this issue Feb 10, 2019 · 5 comments
Assignees
Labels
bug 🐞 Even a perfect program still has bugs
Milestone

Comments

@ali-ramadhan
Copy link
Member

This is probably fine as I don't think the MITgcm uses adaptive time stepping and for what we do I doubt we'll be changing Δt halfway through a simulation, but as it stands if Δt changes it will break some code, read_output(...) methods in particular.

@ali-ramadhan ali-ramadhan added the bug 🐞 Even a perfect program still has bugs label Feb 10, 2019
@glwagner
Copy link
Member

Can you explain? Why does a function like read_output depend on whether the time step is constant? Why does any part of the code need to assume a constant time-step?

I think adaptive time-stepping is useful in many scenarios and the time-step should not be assumed constant in general.

@ali-ramadhan
Copy link
Member Author

You're right, it will be best if the code does not assume a constant time step anywhere.

This probably just affects reading output because the current time is used when naming output files. If the time step is a constant integer then reading the files in order is easy as the file names follow a predictable pattern if you know Δt. If Δt is not constant then you'd have to glob all the files and sort them to process them in order.

@christophernhill suggested to just use the iteration number to name the output files. This would fix this problem, and if a list of times is saved then you know exactly the times that corresponds with each time step.

@glwagner
Copy link
Member

I think iteration number makes more sense.

Also, this strategy should only be employed for "raw" output types like binary or serialized files. Personally, I think 'most' output should use a modern labeled data type like JLD2, NetCDF, HDF5, etc. This is much more sane and promotes reproducible science, and sufficient for almost all cases except specialized cases that require frequent massive data dumps (this seems like a rare use case to me) --- right?

When using labeled data there is no need to name files by iteration number, because you can continue to append new output into a single file and label the appended data within that one file however you wish.

The output functionality might also use Julia's flexibility to allow the user to specify what format they want to use for output, naming, etc.

@ali-ramadhan
Copy link
Member Author

I agree. The focus should be on moving to a proper file format (#30, #31) then this will be a non-issue.

The only use case I see for frequent massive data dumps is if you're running such a massive simulation where writing output almost becomes a bottleneck (apparently was a big issue with LLC4320). No point worrying about this case.

@ali-ramadhan ali-ramadhan added this to the v0.5 milestone Feb 13, 2019
@ali-ramadhan
Copy link
Member Author

This was resolved by de107e1

@ali-ramadhan ali-ramadhan self-assigned this Mar 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 Even a perfect program still has bugs
Projects
None yet
Development

No branches or pull requests

2 participants