Change nematic example to conform to new API for freud 3.0 #68
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
DomFijan
requested review from
yeinlim,
tommy-waltmann and
Lina492375qW1188
and removed request for
a team
|
View / edit / reply to this conversation on ReviewNB tommy-waltmann commented on 2023-05-01T12:30:48Z This is a very wordy way of introducing what the nematic order parameter is. I would remove Eq. 1 and its description, since freud doesn't use it in evaluating the nematic order parameter. Start with some combination of Eqns. 3/4/5 (eq 2 is missing?) and before you even introduce those, say something more conceptual about what it does, why its useful, etc. The citations for the papers with all the complex math can stay, and just say something like "for a more in-depth theoretical treatment of the nematic order parameter, see ..."
In general, this first section needs to do more concept introduction and less mathematical word soup. This is the module introduction. |
|
View / edit / reply to this conversation on ReviewNB tommy-waltmann commented on 2023-05-01T12:30:49Z This paragraph doesn't need to talk about the changes that were made going from version 2 to version 3. Just state what freud needs to do that calculation.
|
|
View / edit / reply to this conversation on ReviewNB tommy-waltmann commented on 2023-05-01T12:30:50Z Similar to what I mentioned above, there doesn't need to be a comment saying everything that has changed from v2 --> v3 |
|
View / edit / reply to this conversation on ReviewNB tommy-waltmann commented on 2023-09-13T16:10:24Z Let's not encourage users to manually compute the nematic OP or nematic director, freud computes those and users can just access the property after calling compute. |
|
View / edit / reply to this conversation on ReviewNB tommy-waltmann commented on 2023-09-13T16:10:26Z It may be useful in a separate example, but I don't think a guide on how to compute smetic order parameters belongs in the module intro for the Nematic order parameter. |
|
View / edit / reply to this conversation on ReviewNB tommy-waltmann commented on 2023-09-25T15:26:26Z This title is what people will see as the title of the example as they look through the documentation. Let's name this "Calculating Smectic Order Parameters".
There are also lots of small grammatical things to fix throughout the entire example notebook. |
|
View / edit / reply to this conversation on ReviewNB tommy-waltmann commented on 2023-09-25T15:26:27Z Typically, we like to put all the helper functions used in an example in their own code block at the top of the example. For examples of this, have a look at the RDF example (here)[https://freud.readthedocs.io/en/latest/gettingstarted/examples/examples/Calculating%20RDF%20from%20GSD%20files.html], and other documentation examples. |
|
View / edit / reply to this conversation on ReviewNB tommy-waltmann commented on 2023-09-25T15:26:28Z Line #13. director = nematic.director / np.linalg.norm(nematic.director) Is the nematic director that freud gives not already normalized? DomFijan commented on 2023-09-29T22:34:12Z We don't normalize it explicitly anywhere. |
|
View / edit / reply to this conversation on ReviewNB tommy-waltmann commented on 2023-09-25T15:26:29Z This title should be changed now that smectic OPs are in a different example. |
|
View / edit / reply to this conversation on ReviewNB tommy-waltmann commented on 2023-09-25T15:26:30Z There are small grammatical issues in a few places still here, too. I can go through and make those changes. |
|
We don't normalize it explicitly anywhere. View entire conversation on ReviewNB |
DomFijan
requested review from
tommy-waltmann
and removed request for
tommy-waltmann
|
View / edit / reply to this conversation on ReviewNB tommy-waltmann commented on 2024-01-31T22:20:05Z I don't understand why the last sentence of this block was here. We don't need to refer people to an intro tutorial to calculating smectic order parameters if we are writing that tutorial. |
|
@DomFijan I have gone through and re-worded the examples in this PR. Have another look over it and let me know if you have any further edits. |
|
@tommy-waltmann |
…-examples into example/nematic-new-api
Description
Due to change in order.Nematic API the example requires changes. In addition many users were confused by the current example. The new example will greatly expand on theory behind the nematic order parameter and introduce an easy way to compute a smectic order parameter as well alongside with some useful references for further reading.
Resolves: #57 and glotzerlab/freud#945
Checklist: