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

function template documentation #42

Open
tkphd opened this issue Jun 13, 2017 · 3 comments
Open

function template documentation #42

tkphd opened this issue Jun 13, 2017 · 3 comments
Assignees

Comments

@tkphd
Copy link
Contributor

tkphd commented Jun 13, 2017

For new users, the use of templated functions in example code may be confusing. Document our usage, explaining

  • where to read more about C++ templates
  • that examples are written as generally as feasible for maximum reusability
  • specification of template parameters in example.hpp files
  • how templates improve readability and reusability while reducing code duplication in our specific examples
  • how a motivated user could approach specializing an example to remove templates in experimental codes
@tkphd
Copy link
Contributor Author

tkphd commented Jun 13, 2017

Addresses issues raised in #41.

@lucentdan
Copy link
Contributor

I like what you've put above. I think it continues enhancing the philosopy of templates. Could you have a look at the tutorial branch? I wonder if this is an alternative or a stepping stone to what you have above? My limited programming/software design experience leaves me feeling that what I've put on the tutorial branch is more like what folks would write when solving a mesoscale modeling problem.

There are other things I would do in the long term like scan a file for input parameters, etc. But this was my natural response to the MMSP library.

What do you think?

@lucentdan lucentdan reopened this Jun 15, 2017
@tkphd
Copy link
Contributor Author

tkphd commented Jun 15, 2017

Thanks for the contribution, @lucentdan. Holistically, the code you've posted seems to fit into an "intermediate tutorial" category, where a user somewhere up the MMSP learning curve would create their own main.cpp and start implementing a streamlined mesoscale model without pulling in unnecessary bits of the library. Looking into the code, it is clearly the "first stab" at an iterative process; worth working on, but not yet ready for prime time. I could envision, at some point, giving an MMSP demo (live or on YouTube) with some derivative of this code as the final step, showing essentially the full complexity achievable by stitching together the foregoing baby-steps. If that makes sense... ?

My main sticking point is, it's unclear how the soul of this contribution fits into the existing MMSP framework. Is it necessary or useful to create a new top-level directory for tutorials? What sets this apart from the existing grain growth examples? How could it fit into the continuous integration (Travis-CI) regime?

Your advanced make usage also makes me wonder about the logical conclusion for "correct" cross-platform builds, using cmake instead of make to streamline installation, testing, coverage checks, compiler variants, etc. It also raises questions about the structure and philosophy of the examples we distribute with MMSP, and issues of example scope creep. Would it make more sense to create a new repository for advanced MMSP examples, or those that break the typical pattern, such as Poisson, convex-splitting Cahn-Hilliard, and this one? What if we moved all the examples into a separate repository? The point is, there are a lot of ways to handle the division between source, documentation, getting-started, and real use cases. We should consider the possibility of deeply refactoring MMSP to make it as clean, easily adoptable, and useful as possible.

Bottom line: there are good nuggets here, and I would like to see the second iteration of this advanced example and discuss where you/we can take this from here.

Since I can't help but get lost in details, here are some specific complaints and suggestions.

  1. Please branch off of develop, not master. In the branching workflow, master only ever gets touched when you fully intend to release a new version.
  2. The name mpf-test is not broadly intelligible: it is unclear what realm of simulations the code belongs to.
  3. There should be no uncertainty in tutorial code. If you believe loops could be switched, do so. If it makes no difference, delete the crufty comment.
  4. Consider breaking the monolithic main function into more easily digestible subroutines.
  5. Consider reading variables in from an input file, rather than from the command line. This would simplify parsing, and help differentiate your program from MMSP.main.hpp. I say this because the code is a more compelling example of advanced usage if you "make it yours." Otherwise... what's the point?
  6. Please stop pressing the "Comment and Close" button, unless the issue really has been resolved to your satisfaction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants