seamplex / wasora

Wasora’s an advanced suite for optimization & reactor analysis
GNU General Public License v3.0
8 stars 4 forks source link

Transient loop considerations #1

Open rvignolo opened 5 years ago

rvignolo commented 5 years ago

Are these lines:

https://github.com/seamplex/wasora/blob/d80ca8aa0ae3612c801592d44d80b5c85c9c54f2/src/wasora.c#L303-L304

correct?

I see the following issues:

  1. Why is ida_step_t_new being set? It is going to be replaced when calling IDASolve.
  2. If I am not mistaken, the second argument for IDASolve should be wasora_var(wasora_special_var(time)) + wasora_var(wasora_special_var(min_dt)), not wasora_var(wasora_special_var(dt)). The User Documentation for IDA says that this argument corresponds to the next time at which a computed solution is desired. However, if the IDA_ONE_STEP mode is set (as in this case), this variable is used on the first call only, and only to get the direction and rough scale of the independent variable. I still believe it should be set as recommended.

Thanks and regards, Ramiro.

gtheler commented 5 years ago

I am over the phone now, but

  1. probably it is needed for the IDASolve call in 304
  2. should not make any difference, feel free to change it and see what happens
rvignolo commented 5 years ago
  1. I will test it because it does not make sense to me. The User Documentation says that this argument corresponds to the time reached by the solver (output).
  2. I believe it does not make any difference, otherwise, wasora would have returned incorrect results during all this time. However, this is not failing because it is being used on the first call only to get the direction. The direction is given by the sign of this argument? This is very subtle. Actually, it would be useful to know if IDA can solve a problem using terminal conditions and negative time steps.
gtheler commented 5 years ago
  1. at the end it contains the time reached by the solver but I think the passed value is used as well I do not remember the details
  2. I am not sura that line is also called the first time, that loop is for the special case in which ida wants so have a dt smaller than wasora, then an internal loop of single steps is made until the wasora dt is attained
rvignolo commented 5 years ago
  1. I have taken a quick glance to the IDASolve source code here and it seems that the value of this parameter is not used.
  2. Yes, I believe I understand what wasora tries to accomplish in each case but, can we discuss this a little bit further? a. What is the following condition trying to catch? https://github.com/seamplex/wasora/blob/d80ca8aa0ae3612c801592d44d80b5c85c9c54f2/src/wasora.c#L290 In the first transient step, ida_step_dt is INFTY so the first condition is never satisfied. However, if a min_dt has been provided and the time equals to zero, we step in. Time equals to zero means first time, right? In this case, wasora can solve a system of DAEs only if the the starting time is zero, right? Should we use wasora_value(wasora_special_var(in_transient_first)) == 1 instead so we can remove the limitation of a starting time equal to zero? If this condition is true, either because we are in the first transient step with a min_dt or because the last computed ida_step_dt is smaller than a min_dt, we perform the loop of single steps that you commented out using a stopping time t + min_dt. When time moves forward, we will keep stepping in if the last computed ida_step_dt is smaller than the min_dt. However, it might happen that IDASolve reaches the stopping time in only one step, meaning that ida_step_dt = min_dt. In this case, the next transient step would not step into this block, but it will go to the next one: https://github.com/seamplex/wasora/blob/d80ca8aa0ae3612c801592d44d80b5c85c9c54f2/src/wasora.c#L315-L326 In this block, there is no stopping time being set using min_dt (there is only one using TIME_PATH keyword), so IDA may advance in time further than wanted, i.e. further than t + min_dt.

Am i missing something?

rvignolo commented 5 years ago

oh I just realized that there is no problem if we go further than t + min_dt, the problem would be if we go further than t + max_dt! That will not happen because: https://github.com/seamplex/wasora/blob/d80ca8aa0ae3612c801592d44d80b5c85c9c54f2/src/wasora.c#L284-L287

The problem would be the opposite, if we advance less than wanted, i.e. we do not reach t + min_dt. Any thoughts?

gtheler commented 5 years ago
  1. ok, remove it and see what happens
  2. about starting at t=0, I once tested setting t0 to something different from zero and it seemed to work, but again, feel free to test and patch anything about the problems you say there are... can you give a MWE which fails?

I am seeinng https://seamplex.com/wasora/doc/realbook/004-exp/ which you can find in the doc/realbook repo and everything works as expected in the exp-dt.was case