mostafa-razavi / ITIC-paper

0 stars 0 forks source link

Fix Figure 2 (algorithm flow-chart) and explain how we solve Eq. 11 (Reviewer 3 Comment 7 ) #31

Open mostafa-razavi opened 5 years ago

mostafa-razavi commented 5 years ago

image

Our current response: As our new derivation in Section 2.1 has modified the numbering of equations, we note that the authors comment regarding Equations 14-17 now pertains to Equations 11-14. We will utilize the updated numbering notation in our response. Figure 2, which is a step-by-step explanation of the algorithm, is helpful when responding to the reviewer’s question. The step “Calculate rhovap (Eq. 11), Psat (Eq. 12)” is performed following the step “Calculate Tsat_new…” Therefore, Tsat is assumed to be a known quantity when solving Eq. 11, such that rhovap is the only unknown. Once rhovap and Psat are computed, the next steps are to “Update Zliq (Eq. 13)” and “Calculate Tsat_new.” Thus, Zliq is also assumed to be constant when solving Eq. 11. Setting Zliq and rhovap to an initial near-zero value does not pertain to solving Eq 11, this refers to the overall iteration process. As mentioned above, Zliq is updated with each iteration. An initial near-zero Zliq value is used to estimate the first iteration of Tsat_new. The initial near-zero value of rhovap is used when solving Eq 11 for the first overall iteration. However, subsequent iterations solve Eq 11 using the previous iteration values for rhovap. Because the iterative ITIC algorithm is not described until Section 2.3, we have modified Section 2.2. to clarify that Equations 11 and 12 can only be solved for a given value of Tsat and Zliq: image We have also modified Figure 2 to clearly distinguish the fixed-point iteration solver for Eq 11 from the overall ITIC iteration process.

Rich's comment in RevLe2:

I also think we could also help address the “unknown Tsat” scenario by modifying Figure 2 slightly. Should we have an outer loop that shows that if the final Tsat deviates strongly from the Tsat_est, we can perform additional simulations. We could include this at the very end of the figure or after the “Calculate Tsat_new from Z vs 1/T” line by including a junction “\delta Tsat < tol?” (or \delta 1/Tsat < tol) (maybe a different tol variable) where if yes you continue as normal but if no you loop back to the first step for improved Tsat_est.

Rich's comment in PDF:

Because there has consistently been confused about fixed-point iterations with rhovap and Eq 11 and how that relates to the overal iterations, I think we should modify this step. Two options: 1) We reword this step to say "Solve Eq 11 for rhovap (e.g., fixed-point iteration)" We should probably separate Calculate Psat (Eq 12) into its own step afterwards 2) We create an entire loop for solving rhovap with fixed-iteration Something like this: Set rhovap_new equal to RHS of Eq 11 computed with rhovap_old Difference between rhovap_new and rhovap_old < tol? Iterate Set rhovap_old = rhovap_new

mostafa-razavi commented 5 years ago

@ramess101 @jrelliottoh

I added a separate step for calculating rho_v in Figure 2 and modified Sections 2.2 and 2.3 quite a bit (This is the new version ITIC-paper.pdf). I think the new narrative is more in line with Figure 2. Let me know what you think so I modify our response accordingly.

image

ramess101 commented 5 years ago

This is looking better. A couple comments: 1) The superscripts “old” and “new” for rhovap need to be non-italics. 2) Unless your modifications removed the entire discussion of fixed-point iteration, we dont just set rhovap = RHS, we actually solve for rhovap using a root-solver, right? 3) Can we have some outer loop for performing new simulations if Tsatest is poor?

On Wednesday, February 6, 2019, mostafa-razavi notifications@github.com wrote:

@ramess101 https://github.com/ramess101 @jrelliottoh https://github.com/jrelliottoh

I added a separate step for calculating rho_v in Figure 2 and modified Sections 2.2 and 2.3 quite a bit (This is the new version). I think the new narrative is more in line with Figure 2. Let me know what you think so I modify our response accordingly.

[image: image] https://user-images.githubusercontent.com/16358113/52389540-edb88d00-2a61-11e9-825a-de3bc2f9d663.png

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mostafa-razavi/ITIC-paper/issues/31#issuecomment-461280708, or mute the thread https://github.com/notifications/unsubscribe-auth/AWUvhOLOznG_sg5mbNJmo4bpmft6y6R2ks5vK6ThgaJpZM4af5vL .

mostafa-razavi commented 5 years ago

1) The superscripts “old” and “new” for rhovap need to be non-italics.

done

2) Unless your modifications removed the entire discussion of fixed-point iteration, we dont just set rhovap = RHS, we actually solve for rhovap using a root-solver, right?

In my code, this is what I do. I do not solve Eq 11 by itself using a root-solver. The whole iterative process converges eventually. I'm not sure we can call it fixed-point method, because it really has too many variables involved whereas the familiar x=g(x) method has one unknown variables. That's why in Figure 5 we have many red curves that each represent RHS of Eq 11 as a function of rho_v at a certain iteration. Using a root-solver could be an alternative though.

3) Can we have some outer loop for performing new simulations if Tsatest is poor?

I responded to this in the commented PDF that you sent me. I'll send my response soon. Here is my response. I think that would complicate this algorithm too much. I think we have satisfied the reviwers by giving a lot of attention to the case where good Tsat estimates are not available.

ramess101 commented 5 years ago

The “new” and “old” superscripts in the diamond with “ < tol” are still italicized.

OK, I had misunderstood what you were referring to as the fixed-point method. I thought this was just solving the rhovap equation since it is implicit for rhovap. I might have confused the reviewer with some of my responses.

It looks like you still use the term “fixed point” in the Figure 5 caption, did you mean to eradicate that term?

OK, I agree that the figure could be cluttered. But I think the Next IC loop could also be removed.

On Thursday, February 7, 2019, mostafa-razavi notifications@github.com wrote:

  1. The superscripts “old” and “new” for rhovap need to be non-italics.

done

  1. Unless your modifications removed the entire discussion of fixed-point iteration, we dont just set rhovap = RHS, we actually solve for rhovap using a root-solver, right?

In my code, this is what I do. I do not solve Eq 11 by itself using a root-solver. The whole iterative process converges eventually. I'm not sure we can call it fixed-point method, because it really has too many variables involved whereas the familiar x=g(x) method has one unknown variables. That's why in Figure 5 we have many red curves that each represent RHS of Eq 11 as a function of rho_v at a certain iteration. Using a root-solver could be an alternative though.

  1. Can we have some outer loop for performing new simulations if Tsatest is poor?

I responded to this in the commented PDF that you sent me. I'll send my response soon. Here is my response. I think that would complicate this algorithm too much. I think we have satisfied the reviwers by giving a lot of attention to the case where good Tsat estimates are not available.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mostafa-razavi/ITIC-paper/issues/31#issuecomment-461310765, or mute the thread https://github.com/notifications/unsubscribe-auth/AWUvhI2xmh2MKjCMatdfpYOE7EGThW_oks5vK9GCgaJpZM4af5vL .

mostafa-razavi commented 5 years ago

OK, I agree that the figure could be cluttered. But I think the Next IC loop could also be removed.

Why not just one final loop that says: Tsat - Tsat_est < tol ? And it goes back to the very beginning if the answer is no. We could have this as a diamond that determines whether or not you go to the next IC and remove the "Report Psat...": rhovap^new - rhovap^old < tol? yes Tsat - Tsat_est < tol? if yes Next IC if no Repeat ITIC process

I can't seem to find a proper way to add another loop for Tsat_est. Going back to the beginning of the flowchart if NO implies that we need to run simulations at all state points, but it's not true, we only need to run simulations at middle points of IC's and new Tsat_est.

ramess101 commented 5 years ago

@mostafa-razavi

First, I just noticed that in the first box, "Determine rho and T values" there should should probably be two steps that say, "Estimate T_c", "Calculate T_IT = 1.2 T_c"

ramess101 commented 5 years ago

@mostafa-razavi

I can't seem to find a proper way to add another loop for Tsat_est. Going back to the beginning of the flowchart if NO implies that we need to run simulations at all state points, but it's not true, we only need to run simulations at middle points of IC's and new Tsat_est.

What about something like this (might need to change the wording to make the text fit in the bubbles):

image

ramess101 commented 5 years ago

@mostafa-razavi

Note that the Tsat - Tsat_est criteria might be better expressed as 1/Tsat - 1/Tsat_est. Still trying to think that one through, probably doesn't make a huge difference though.

mostafa-razavi commented 5 years ago

This will work. If we go for it, we need to change the manuscript where we talk about Tsat_est and maybe in the results section where we discuss the rare molecule.

ramess101 commented 5 years ago

@mostafa-razavi

So I just realized that this is not really what we show in the rare molecule section. In that section we don't perform the entire ITIC analysis to determine Tsat_est and then repeat NVT simulations. Instead, we just extrapolate/interpolate a few isochore NVT results to get Tsat_est. Right? Of course, we could have run the ITIC analysis to completion, and we probably would've gotten similar Tsat_est values...

ramess101 commented 5 years ago

@mostafa-razavi

Also, if we do decide to modify this figure, you could move the new NVT simulations bubble up to take advantage of the white space there:

image

mostafa-razavi commented 5 years ago

@ramess101 We do mention the Tsat_est iteration in a whole paragraph in Section 7 as an alternative. But I'm just afraid that changing too much will trigger more and more questions from the reviewers. I really hoe this is the last round of reviews :)

ramess101 commented 5 years ago

@mostafa-razavi

Yeah, me too. My intent was to satisfy the reviewer's questions about Tsat iterations, etc. But if this doesn't actually represent how we would iterate on Tsat_est, then we shouldn't change a lot of the discussion just to make this figure more accurate.

But you tell me, do you think this is an accurate representation of how we propose iterating on Tsat_est for a poor initial guess? Or would something like this be more consistent with how you analyzed the rare aromatic:

image

mostafa-razavi commented 5 years ago

@ramess101 The above flow-chart is is very close to what I did in practice. However, doesn't the loop on T_sat imply that we might need to do several iterations, hence several rounds of simulations on Tsat_est and middle points on IC? In practice, even with horrible Tsat guesses with a linear extrapolation/interpolation, you can quickly get a good estimate for Tsat. But for the sake of completion, I'm all for the above flow-chart. To make it look better, maybe combine the two oval steps and squeeze them into the empty space next to the blue diamond?

mostafa-razavi commented 5 years ago

@ramess101

First, I just noticed that in the first box, "Determine rho and T values" there should should probably be two steps that say, "Estimate T_c", "Calculate T_IT = 1.2 T_c"

I think we should really assume that the user has access to experimental values of T_c. I agree that experimental values of T_c is also an estimate, but I don't want to overuse the word estimate, because T_c really doesn't play any role in ITIC except for making sure isotherm is supercritical.

I did break down the first box though. I'll send when I'm done

mostafa-razavi commented 5 years ago

@ramess101

My other concern about the above figure is that we do n't really check the blue diamond condition in the rho_v loop. This might become very confusing for the reader.

ramess101 commented 5 years ago

@mostafa-razavi

I think we just need to say in the text that because extrapolation/interpolation is quite reliable, the tolerance for T can be as great as 10-20 K (?)

We could say that typically the Tsat tolerance criteria is satisfied without any iterations.

Combining the new bubbles into the open white space sounds good. I just liked having the NVT simulations close to each other so it is clear that it would require additional simulations.

If you don’t say estimate Tc, I think you should say “Estimate T_IT /approx 1.2 Tc”

Yeah having Tsat loop inside rhov loop isnt the best if you think the other format is an accurate representation.

On Friday, February 8, 2019, mostafa-razavi notifications@github.com wrote:

@ramess101 https://github.com/ramess101 The above flow-chart is is very close to what I did in practice. However, doesn't the loop on T_sat imply that we might need to do several iterations, hence several rounds of simulations on Tsat_est and middle points on IC? In practice, even with horrible Tsat guesses a linear extrapolation/interpolation, you can quickly get a good estimate for Tsat. But for the sake of completion, I'm all for the above flow-chart. To make it look better, maybe combine the two oval steps and squeeze them into the empty space next to the blue diamond?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mostafa-razavi/ITIC-paper/issues/31#issuecomment-461965740, or mute the thread https://github.com/notifications/unsubscribe-auth/AWUvhHOcfcMVp-a3DEQP_51kpILuR-JBks5vLfcxgaJpZM4af5vL .

mostafa-razavi commented 5 years ago

image

The problem with the above flowchart is that we actually do not update all Tsat values; we only update the Tsat corresponding to the current IC. Also, the step "Calculate B2..." is redundant in the second Tsat iteration because we do that once.

I made some changes and came up with the following flow-chart (left), but I really think adding Tsat iteration makes the flowchart unnecessarily complicated. I prefer not adding Tsat iteration at all; something like the right flowchart

image

ramess101 commented 5 years ago

@mostafa-razavi

Yeah, I was thinking it would be better to just say update Tsat_est since we don’t update all of T11,13,15,17,19, but I didnt know if we had a term like Tmid for the other point. I like the version on the left, although I think there are a few changes that could be made. Also, I don’t think it’s a big deal to be redundant about B2.

On Saturday, February 9, 2019, mostafa-razavi notifications@github.com wrote:

[image: image] https://user-images.githubusercontent.com/23408516/52507702-8c063900-2baf-11e9-8c57-8e0d1caeaeac.png

The problem with the above flowchart is that we actually do not update all Tsat values; we only update the Tsat corresponding to the current IC. Also, the step "Calculate B2..." is redundant in the second Tsat iteration because we do that once.

I made some changes and came up with the following flow-chart (left), but I really think adding Tsat iteration makes the flowchart unnecessarily complicated. I prefer not adding Tsat iteration at all; something like the right flowchart

[image: image] https://user-images.githubusercontent.com/16358113/52528512-8b080100-2cae-11e9-9aef-e836c9b88ff2.png

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mostafa-razavi/ITIC-paper/issues/31#issuecomment-462096973, or mute the thread https://github.com/notifications/unsubscribe-auth/AWUvhCpGezuoobh8zZ5D33JpEhypFOgWks5vL366gaJpZM4af5vL .