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

Move // comments to be /// #2

Closed
dsyme opened this issue Dec 5, 2015 · 1 comment
Closed

Move // comments to be /// #2

dsyme opened this issue Dec 5, 2015 · 1 comment

Comments

@dsyme
Copy link
Collaborator

dsyme commented Dec 5, 2015

The library is light on comments (though there are some! And the code is readable).

One easy way to improve things is to move // comments like these

| Constant    of D         // Constant
| Decay       of D * D     // 1 / t decay, a = a0 / (1 + kt). Initial value, decay rate
| ExpDecay    of D * D     // Exponential decay, a = a0 * Exp(-kt). Initial value, decay rate

to be /// commments like these

/// Constant learning rate
| Constant    of D         

/// Learning rate of 1 / t decay, a = a0 / (1 + kt). Initial value, decay rate
| Decay       of D * D    

/// Exponential decay learning rate, a = a0 * Exp(-kt). Initial value, decay rate
| ExpDecay    of D * D  

etc. Since you've got the comments there already you may as well propagate them to the user through /// comments. In general having a /// comment on every type, member, union case and record field is a good thing.

The same applies to DiffSharp - again readable once you know the AD techniques but more comments would be helpful, for example union cases D and DM etc. don't have commments

@gbaydin
Copy link
Collaborator

gbaydin commented Dec 5, 2015

Yes, this is a good point.

I've been experimenting with this code for a while in our lab and I made a sudden decision to make it public recently for demonstrating our ideas. Comments are missing in many places and it's definitely on my list to improve this soon. I do care a lot about documentation and readability.

Thank you for pointing out I can put /// comments on union cases. I didn't know.

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