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

Make default of Number of grouped files 16 #1649

Merged
merged 1 commit into from May 15, 2017

Conversation

jaustermann
Copy link
Contributor

I would prefer the default to be 1 ... what do others think?

@bangerth
Copy link
Contributor

Won't that kill the possibility of parallel rendering of viz data?

Maybe more to the point, what is the problem you have with it?

@tjhei
Copy link
Member

tjhei commented May 14, 2017

A good default is probably one file per node you run on, but that is difficult to put into this parameter. I agree that "0" rarely makes sense.

@jaustermann
Copy link
Contributor Author

If there is one file per timestep instead of say a few hundred it makes it easier to see what has been run plus there are fewer files to transfer from the cluster. For a new user it might also be less confusing to have one file per timestep. It does kill the possibility of parallel rendering of viz data but I never use that and if that's what someone wants they can always change the default. I don't feel strongly about this change, just a suggestion.

@gassmoeller
Copy link
Member

Actually, I would prefer to keep it at 0. It is true that creating hundreds of files per timestep is annoying, however the alternative seems worse to me (namely creating one file on a big cluster-run that you are then unable to read, because no available machine has enough memory). I agree this is a case that rarely happens, but if it happens a lot of computing time will be wasted. One could argue that users running these large models should know what they are doing, but in reality I am sure a PhD student will run into this issue. @jaustermann did you had problems with the large number of files? I once ran into our clusters file limit, but mostly because I had many models lying around.

@tjhei
Copy link
Member

tjhei commented May 14, 2017

We could set it to something like 16, which will still allow you to do parallel viz and avoid producing hundreds of files per timestep.

@bangerth
Copy link
Contributor

16 seems too small to me. If you have a big model, you'd want there to be 64 or 128 files.

@gassmoeller
Copy link
Member

I often use a parameter of 16 files, it seems like a reasonable compromise to me. If you need more, you likely have a model exceeding a billion dofs, in which case you really should know what you are doing.

What happens if there are less processors in the run than the number of grouped files? Will it simply write the smaller number of files?

@bangerth
Copy link
Contributor

That seems like reasonable behavior -- in other words, if it doesn't automatically happen that we write only up to as many files as there are processors, then that is what ought to happen.

@tjhei
Copy link
Member

tjhei commented May 14, 2017

We do std::min(group_files,n_processes) in the code, so I think it should work, but it would be great to quickly test this with 1, <16, =16, >16 procs. /run-tests

@jaustermann
Copy link
Contributor Author

I think this would be a good solution as well, I will test it to be sure.

@jaustermann
Copy link
Contributor Author

Did a couple of tests on different numbers of cores and it behaves as expected.

@jaustermann jaustermann changed the title Make default of Number of grouped files 1 Make default of Number of grouped files 16 May 15, 2017
@bangerth
Copy link
Contributor

@jaustermann -- please add a changelog entry for this!

@jaustermann
Copy link
Contributor Author

done!

@tjhei tjhei merged commit d3318d3 into geodynamics:master May 15, 2017
@jaustermann jaustermann deleted the num_grouped_files branch June 22, 2018 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants