Add comments
This commit is contained in:
parent
628f7a7db6
commit
a0d3a39d07
57
review.md
57
review.md
@ -23,7 +23,8 @@ Nothing
|
|||||||
[x] 10: "Magnetic materials can be classified" => "Magnetic materials are classified". I changed this, since they are actually classified
|
[x] 10: "Magnetic materials can be classified" => "Magnetic materials are classified". I changed this, since they are actually classified
|
||||||
[x] 11: consist => consists. "One of the groups" is singular.
|
[x] 11: consist => consists. "One of the groups" is singular.
|
||||||
[ ] 12: Is the saturation optional, or will they become saturated when exposed to a magnetic field?
|
[ ] 12: Is the saturation optional, or will they become saturated when exposed to a magnetic field?
|
||||||
Valid point, they do become saturated so maybe rephrase
|
- janitaws: Valid point, they do become saturated so maybe rephrase
|
||||||
|
- coryab: Just remove "can", and put a comma after "field" and it should be good
|
||||||
[x] 26: "as well as algorithms ..." => "as well as the algorithms ..."
|
[x] 26: "as well as algorithms ..." => "as well as the algorithms ..."
|
||||||
[x] 28: "we conclude ..." => "we will conclude ..."
|
[x] 28: "we conclude ..." => "we will conclude ..."
|
||||||
|
|
||||||
@ -34,8 +35,9 @@ Valid point, they do become saturated so maybe rephrase
|
|||||||
[x] 7: "the length of the lattice ..." => "the length of a lattice ...". We're not talking about a specific lattice.
|
[x] 7: "the length of the lattice ..." => "the length of a lattice ...". We're not talking about a specific lattice.
|
||||||
[x] 26: remove "will", as we are assuming a 2x2 now and not later in the paper.
|
[x] 26: remove "will", as we are assuming a 2x2 now and not later in the paper.
|
||||||
[x] 33: denote => denotes. $\langle k l \rangle$ is singular.
|
[x] 33: denote => denotes. $\langle k l \rangle$ is singular.
|
||||||
[ ] 34: Change to present tense, as we are considering it in this section and not later in the paper.
|
[x] 34: Change to present tense, as we are considering it in this section and not later in the paper.
|
||||||
I think I've fixed it, however, tense is apparently not my jam these days XP
|
- janitaws: I think I've fixed it, however, tense is apparently not my jam these days XP
|
||||||
|
- coryab: Looks good! I'll mark it as done.
|
||||||
[x] 92: "Values of total energy ..." => "Values of the total energy ..."
|
[x] 92: "Values of total energy ..." => "Values of the total energy ..."
|
||||||
[x] 98: "by number of spins ..." => "by the number of spins ..."
|
[x] 98: "by number of spins ..." => "by the number of spins ..."
|
||||||
[x] 162: Why are the equations for heat capacity and susceptibility in different environments?
|
[x] 162: Why are the equations for heat capacity and susceptibility in different environments?
|
||||||
@ -53,21 +55,27 @@ I agree, it does make more sense
|
|||||||
[x] 239: require => requires. "generating new random states" is singular.
|
[x] 239: require => requires. "generating new random states" is singular.
|
||||||
[x] 240: detaild => detailed
|
[x] 240: detaild => detailed
|
||||||
[x] 241: criterias => criteria. the plural of criterion is criteria.
|
[x] 241: criterias => criteria. the plural of criterion is criteria.
|
||||||
[x] 242: Figure => Algorithm, since we can remove the figure environment enclosing the algorithm environment.
|
[x] 242: Figure => Algorithm, since we can remove the figure environment enclosing the algorithm environment. More details are specified in the next comment below.
|
||||||
[ ] More details are specified in the next comment below.
|
|
||||||
[ ] 244: A Monte Carlo cycle consists of attempts of flipping a spin, and not a single attempt to flip a spin.
|
[ ] 244: A Monte Carlo cycle consists of attempts of flipping a spin, and not a single attempt to flip a spin.
|
||||||
Not sure how to rephrase the sentence to make it correct, any specific suggestions?
|
- janitaws: Not sure how to rephrase the sentence to make it correct, any specific suggestions?
|
||||||
|
- coryab: It depends. If we want to describe a Monte Carlo cycle, then something like "One Monte Carlo cycle consists of $N$ attempted spin flips." would work. If we want to give a word to a single attempt at a spin flip, then maybe "A single iteration of the Metropolis-Hastings algorithm consists of attempting to flip a spin in the lattice chosen randomly."?
|
||||||
[x] 261: Instead of having the algorithm inside a figure, you can use the H specifier in the algorithm environment (which you already do). You can read more about it [here](https://tex.stackexchange.com/questions/231191/algorithm-in-revtex4-1). I have already tested that it works, and it makes it so we can actually refer to it as an algorithm and not a figure.
|
[x] 261: Instead of having the algorithm inside a figure, you can use the H specifier in the algorithm environment (which you already do). You can read more about it [here](https://tex.stackexchange.com/questions/231191/algorithm-in-revtex4-1). I have already tested that it works, and it makes it so we can actually refer to it as an algorithm and not a figure.
|
||||||
This might have fallen through the cracks for me since I just copy-pasted from metropolis.tex
|
- janitaws: This might have fallen through the cracks for me since I just copy-pasted from metropolis.tex
|
||||||
|
- coryab: Yeah, now that I think about it, I copied this directly from the example which had this hacky solution. RevTex is honestly really bad imo.
|
||||||
[x] 284: reach => reaches. "The Markov process" is singular.
|
[x] 284: reach => reaches. "The Markov process" is singular.
|
||||||
[ ] 283: I changed the paragraph to include a better introduction of the concept burn-in time, do you think the "flow" is better now?
|
[x] 283: I changed the paragraph to include a better introduction of the concept burn-in time, do you think the "flow" is better now?
|
||||||
|
- coryab: Looks good!
|
||||||
[x] 295: implementation => execution. Implementation is the writing of the code, while the runtime is during execution of the program. I would also say to avoid potentially having the program run endlessly.
|
[x] 295: implementation => execution. Implementation is the writing of the code, while the runtime is during execution of the program. I would also say to avoid potentially having the program run endlessly.
|
||||||
[ ] 292: Do you have a good way of including "I would also say to avoid potentially having the program run endlessly" in the sentence?
|
[ ] 292: Do you have a good way of including "I would also say to avoid potentially having the program run endlessly" in the sentence?
|
||||||
[ ] 297: Change of tense. We have used present tense for most of this section, so we should continue with that.
|
- coryab: Replace the sentence with "In addition, we set a tolerance to verify convergence, and a maximum number of Monte Carlo cycles to avoid potentially having the program run indefinitely."?
|
||||||
Added this into a new point, since the line numbers have shifted now
|
[x] 297: Change of tense. We have used present tense for most of this section, so we should continue with that.
|
||||||
[ ] 294: So for the paragraph starting with "We used a pattern..." should be rewritten to present tense?
|
- janitaws: Added this into a new point, since the line numbers have shifted now
|
||||||
|
- coryab: Ok, will mark this as OK.
|
||||||
|
[x] 294: So for the paragraph starting with "We used a pattern..." should be rewritten to present tense?
|
||||||
|
- coryab: After reading it a bit better, it makes more sense to have the implementation part in past tense, so I'll mark it as OK.
|
||||||
[ ] 300: Lack of if-tests does not hinder parallelization, but it can hinder compiler optimizations.
|
[ ] 300: Lack of if-tests does not hinder parallelization, but it can hinder compiler optimizations.
|
||||||
Suggested change "... avoids the use of if-tests, so we can take advantage of the compiler optimization."
|
- janitaws: Suggested change "... avoids the use of if-tests, so we can take advantage of the compiler optimization."
|
||||||
|
- coryab: Yes, that works great!
|
||||||
[x] 318: used => use. Keep to the present tense for consistency.
|
[x] 318: used => use. Keep to the present tense for consistency.
|
||||||
[x] 320: used => use.
|
[x] 320: used => use.
|
||||||
[x] 320: "the Python ..." => "the Python ...". Double space.
|
[x] 320: "the Python ..." => "the Python ...". Double space.
|
||||||
@ -80,20 +88,24 @@ Suggested change "... avoids the use of if-tests, so we can take advantage of th
|
|||||||
|
|
||||||
[x] 15: Should have a line break to indicate new paragraph.
|
[x] 15: Should have a line break to indicate new paragraph.
|
||||||
[ ] 15: It might be relevant, but it's a lot more work. we have all the tools necessary to produce the plots, so it depends on how much work we can do.
|
[ ] 15: It might be relevant, but it's a lot more work. we have all the tools necessary to produce the plots, so it depends on how much work we can do.
|
||||||
I suggest we leave it out for now, I'll comment it out in case we want to use it
|
- janitaws: I suggest we leave it out for now, I'll comment it out in case we want to use it
|
||||||
|
- coryab: Sounds good! I'll leave this as undone for now until we decide fully on what to do.
|
||||||
[x] 89: onordered => unordered
|
[x] 89: onordered => unordered
|
||||||
[ ] 97: More accurately, we generated 10M samples per temperature point.
|
[ ] 97: More accurately, we generated 10M samples per temperature point.
|
||||||
True, so "...$10$ million samples of spin configurations for lattices..." => "...$10$ million samples of spin configurations, per temperature, for lattices..."
|
- janitaws: True, so "...$10$ million samples of spin configurations for lattices..." => "...$10$ million samples of spin configurations, per temperature, for lattices..."
|
||||||
[ ] 77: Approve change "Histogram $T = 1.0 J / k_{B}$" => "Distribution of values of energy per spin, when temperature is $T = 1.0 J / k_{B}$"
|
- coryab: That sounds good!
|
||||||
[ ] 85: Approve change "Histogram $T = 2.4 J / k_{B}$" => "Distribution of values of energy per spin, when temperature is $T = 1.0 J / k_{B}$"
|
[x] 77: Approve change "Histogram $T = 1.0 J / k_{B}$" => "Distribution of values of energy per spin, when temperature is $T = 1.0 J / k_{B}$"
|
||||||
|
[x] 85: Approve change "Histogram $T = 2.4 J / k_{B}$" => "Distribution of values of energy per spin, when temperature is $T = 1.0 J / k_{B}$"
|
||||||
[x] 100: Should be that the program allocated, so that the reader understands that it does the division automatically.
|
[x] 100: Should be that the program allocated, so that the reader understands that it does the division automatically.
|
||||||
I agree!
|
I agree!
|
||||||
[x] 101: thread => threads
|
[x] 101: thread => threads
|
||||||
[ ] 105: We used a profiler. And it's not just for ensuring load balance among threads, it's also for each process, and we use it to see that data transfer is minimal.
|
[ ] 105: We used a profiler. And it's not just for ensuring load balance among threads, it's also for each process, and we use it to see that data transfer is minimal.
|
||||||
So instead of "We ran a profiler" it should be "We used a profiler"? Could you suggest a way of formulating the sentence so that the info is correct?
|
- janitaws: So instead of "We ran a profiler" it should be "We used a profiler"? Could you suggest a way of formulating the sentence so that the info is correct?
|
||||||
|
- coryab: Yes to the first point. "To check if the parallelization was optimal, we used a profiler, which found that the program was efficient. The thing that scored slightly less was the MPI load balance, which is most likely because the master process gathered all data using blocking communication, which made the other processes wait, in addition to having one process work more."
|
||||||
[ ] 106: was => were
|
[ ] 106: was => were
|
||||||
My previous comment might make this unnecessary?
|
- janitaws: My previous comment might make this unnecessary?
|
||||||
[ ] 113: Included description of variables in caption
|
- coryab: If you use my suggestion, then this can be checked off at the same time.
|
||||||
|
[x] 113: Included description of variables in caption
|
||||||
[x] 116: deacrese => decrease
|
[x] 116: deacrese => decrease
|
||||||
[x] 117: "suggesting the system ..." => "suggesting that the system ...".
|
[x] 117: "suggesting the system ..." => "suggesting that the system ...".
|
||||||
[x] 118: increase => increases. "The system energy" is singular.
|
[x] 118: increase => increases. "The system energy" is singular.
|
||||||
@ -105,7 +117,7 @@ My previous comment might make this unnecessary?
|
|||||||
## ./latex/sections/conclusion.tex
|
## ./latex/sections/conclusion.tex
|
||||||
|
|
||||||
[x] 14: Didn't we arrive at 5000?
|
[x] 14: Didn't we arrive at 5000?
|
||||||
Haha, ofc we did!
|
- janitaws: Haha, ofc we did!
|
||||||
[x] 22: temperature => temperatures
|
[x] 22: temperature => temperatures
|
||||||
[x] 24: variation => variance
|
[x] 24: variation => variance
|
||||||
[x] 26: variation => variance
|
[x] 26: variation => variance
|
||||||
@ -113,8 +125,7 @@ Haha, ofc we did!
|
|||||||
## ./latex/sections/appendices.tex
|
## ./latex/sections/appendices.tex
|
||||||
|
|
||||||
[x] 95: "The squared energy" => "The squared energy and magnetization functions are then"
|
[x] 95: "The squared energy" => "The squared energy and magnetization functions are then"
|
||||||
I agree, that sound a lot better! I just ended up wtiting something somewhat descriptive in lack of words.
|
- janitaws: I agree, that sound a lot better! I just ended up wtiting something somewhat descriptive in lack of words.
|
||||||
[x] 130: he => the
|
[x] 130: he => the
|
||||||
Also, I changed the Eq. => to Equation
|
- janitaws: Also, I changed the Eq. => to Equation
|
||||||
|
|
||||||
[x] 188: General, Included captions for figures in section
|
[x] 188: General, Included captions for figures in section
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user