Differentiation Infrastructure in Deepchem Tutorial #3912
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
| @@ -0,0 +1,192 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,252 @@ | |||
| { | |||
There was a problem hiding this comment.
Add a general introduction here. Provide general scientific context. What are we adding to DeepChem and why?
Also, please coordinate with Rida to have her do a pass through the tutorial for writing style help
Reply via ReviewNB
| @@ -0,0 +1,252 @@ | |||
| { | |||
There was a problem hiding this comment.
The font here is weird; can you make it the usual font for the tutorials?
Make the title "Broyden's First Method" and not "Broyden1 method." Make sure capitalization etc is correct on titles for sections
Reply via ReviewNB
There was a problem hiding this comment.
i was using html tags, i will revert back to normal.
| @@ -0,0 +1,417 @@ | |||
| { | |||
There was a problem hiding this comment.
examples/tutorials/Differentiation_Infrastructure_in_Deepchem.ipynb
Outdated
Show resolved
Hide resolved
examples/tutorials/Differentiation_Infrastructure_in_Deepchem.ipynb
Outdated
Show resolved
Hide resolved
examples/tutorials/Differentiation_Infrastructure_in_Deepchem.ipynb
Outdated
Show resolved
Hide resolved
examples/tutorials/Differentiation_Infrastructure_in_Deepchem.ipynb
Outdated
Show resolved
Hide resolved
examples/tutorials/Differentiation_Infrastructure_in_Deepchem.ipynb
Outdated
Show resolved
Hide resolved
|
@rida151 @aaronrockmenezes I have done the recommended fixes, for anymore improvement please tell. @rbharath Ready For Review. |
|
LGTM |
| @@ -0,0 +1,529 @@ | |||
| { | |||
There was a problem hiding this comment.
@GreatRSingh Can you add this to the rendering order so it gets rendered on the website?
| @@ -0,0 +1,527 @@ | |||
| { | |||
There was a problem hiding this comment.
Nit: fix capitalization on the title.
Let's remove the part about advancing scientific knowledge etc; too vague and doesn't teach the reader concrete skills
Reply via ReviewNB
| @@ -0,0 +1,527 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,527 @@ | |||
| { | |||
There was a problem hiding this comment.
Explain the types of deep architectures these primitives could be useful in. For example, DFT. I think there are also deep implicit methods that use root finding as part of the architecture. Also some of the neural ODE style papers. Add citations. Be explicit and precise
Reply via ReviewNB
There was a problem hiding this comment.
LGTM, good to merge once CI is clear.
@GreatRSingh is this safe to merge in (no CI failures etc)?
Yes its safe to merge. There is a mypy failure but its not related to this PR. |
Description
Fix #(issue)
Type of change
Please check the option that is related to your PR.
Checklist
yapf -i <modified file>and check no errors (yapf version must be 0.32.0)mypy -p deepchemand check no errorsflake8 <modified file> --countand check no errorspython -m doctest <modified file>and check no errors