time loop conditional in monolithic.py

[keiyamamo]

Inside monolithic.py, we have a conditional for time loop as follows.

turtleFSI/turtleFSI/monolithic.py

Line 156 in 0b95d44

while t <= T + dt / 10 and not stop: # + dt / 10 is a hack to ensure that we take the final time step t == T

I think + dt/10 should really be -dt/10 to ensure t==T because adding dt/10 makes it go into while loop even when t==T and this will end up t=T+dt as the final time step.

I wanna make sure that others agree on this, as this is important part of turtle.
Could I have your opinion on this?
@johannesring @jorgensd @dbruneau-mie

[dbruneau-mie]

Hey Kei - I do think the time loop needs some work, and this is one aspect that would be nice to fix. It seems to be fairly independant of the rest of the solver, but I am not positive. Multiple parts of the solver and post processing depend on the time loop and it is easy to break it, so I would recommend testing it it you want to take this on.

Another consideration with the time loop is ensuring even temporal spacing in the visualization file after a restart when save step >=2 is used. I found a fix for this but it is not currently incorporated in the time loop, so if one restarts the solver there will be uneven time spacing between the visualization steps. In general, I think the order of things in the time loop is a bit strange currently, and could use some work; I also think "counter" should be in sync with "dt". We can discuss this more in our meeting on Thursday.

[keiyamamo]

Hi David,

Thanks for the input! I agree with you that it is fairly independent of the rest of the solver, but also have to be really careful when it comes to fixing time loop.
For the particular problem I mentioned, I did a very simple test where I set dt=0.5 and T=3 and this resulted in

Solved for timestep 7, t = 3.5000 in 0.9 s

while fixing the conditional to while t <= T - dt / 10 and not stop: resulted in

Solved for timestep 6, t = 3.0000 in 0.9 s

Therefore, assuming that turtle is printing out the correct information, I’m pretty confident that the conditional should be fixed as I proposed. Yet, I will look into a bit more to make sure that this change won’t affect the solver.

Regarding the counter, I think counter gets mixed up after a restart, so that should also be fixed.

[keiyamamo]

Another issue with current implementation of time loop is that we save the visualization at the first time step t=dt due to the definition of counter.

turtleFSI/turtleFSI/monolithic.py

Lines 181 to 186 in 0b95d44

	# Store results
	if counter % save_step == 0:
	vars().update(save_files_visualization(**vars()))
	# Update the time step counter
	counter += 1

Because the counter is incremented after we call save_files_visualization, if counter % save_step == 0: becomes True at the first time loop no matter what value we give to save_step.

For example, if we set T=1, dt=0.1, and save_step=2, I would expect the visualization file to contain the results from dt=0.2, 0.4, 0.6, 0.8, 1.0 but current implementation gives dt=0.1, 0.3, 0.5, 0.7, 0.9, which is not intuitive for me.

[KristianValen-Sendstad]

Hi Kei, You raise very relevant issues. WOuld be good to discuss with the rest of the group later today. KVS

…

On Thu, 16 Mar 2023 at 10:59, Kei Yamamoto ***@***.***> wrote: Another issue with current implementation of time loop is that we save the visualization at the first time step t=dt due to the definition of counter . https://github.com/KVSlab/turtleFSI/blob/0b95d443e3d694a118c1aab16599a7b58c5118e5/turtleFSI/monolithic.py#L181-L186 Because the counter is incremented after we call save_files_visualization, if counter % save_step == 0: becomes True at the first time loop no matter what value we give to save_step. For example, if we set T=1, dt=0.1, and save_step=2, I would expect the visualization file to contain the results from dt=0.2, 0.4, 0.6, 0.8, 1.0 but current implementation gives dt=0.1, 0.3, 0.5, 0.7, 0.9, which is not intuitive for me. — Reply to this email directly, view it on GitHub <#42 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA4X54PTR2LOWKELCQHVOKLW4LQALANCNFSM6AAAAAAV2YUMGI> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>