search

SLACKHA / pyJac

Creates C and CUDA analytical Jacobians for chemical kinetics ODE systems
http://slackha.github.io/pyJac/
MIT License
52 stars 23 forks source link

Bug in `dydt` for `cv_avg` #43

Open mgoodson-cvd opened 2 years ago

mgoodson-cvd commented 2 years ago

Just a warning to anyone still using this: there is a bug in dydt.c in the function computing cv_avg, here.

https://github.com/SLACKHA/pyJac/blob/5f54b2d6c04e1ee62d9db8bbf7553ad785f58c33/pyjac/core/rate_subs.py#L2431

There needs to be a "+" sign prior to opening the next parentheses, otherwise you end up with code that looks like this:

  double cv_avg = (cv[0] * y[1]) + (cv[1] * y[2]) + (cv[2] * y[3]) + (cv[3] * y[4])(cv[4] * y_N);
skyreflectedinmirrors commented 2 years ago

Heh, you can see we fixed it for conp here: https://github.com/SLACKHA/pyJac/blob/5f54b2d6c04e1ee62d9db8bbf7553ad785f58c33/pyjac/core/rate_subs.py#L2281

skyreflectedinmirrors commented 2 years ago

I think this should do it? https://github.com/SLACKHA/pyJac/pull/44

I haven't run this in awhile, so if you test it and it looks good, let me know and I'll merge it in.

mgoodson-cvd commented 2 years ago

@arghdos Yes, that fixed it! Thanks for the quick response.

Once I fixed that, the compiler found another bug here: https://github.com/SLACKHA/pyJac/blob/5f54b2d6c04e1ee62d9db8bbf7553ad785f58c33/pyjac/core/rate_subs.py#L2362

There should be a comma before the rho so that the call the eval_conc_rho is correct. After I made that change manually, it compiled!