Skip to content

On-the-fly interpolation of collision coeffs.#135

Merged
allegroLeiden merged 5 commits intolime-rt:masterfrom
tlunttil:coll_coeffs
Jul 7, 2016
Merged

On-the-fly interpolation of collision coeffs.#135
allegroLeiden merged 5 commits intolime-rt:masterfrom
tlunttil:coll_coeffs

Conversation

@tlunttil
Copy link
Copy Markdown
Contributor

@tlunttil tlunttil commented Jun 9, 2016

Implementation of the on-the-fly interpolation of collisional
coefficients that was discussed in comments to issue #81. With the
default example/model.c memory saving is about 50% and there is no
effect on speed.

tlunttil and others added 2 commits June 9, 2016 17:08
This commit implements the on-the-fly interpolation of collisional
coefficients that was discussed in comments to issue #81. With the
default example model memory saving is about 50% and there is no
effect on speed.
@smaret smaret added this to the Release 1.6 milestone Jun 13, 2016
@allegroLeiden
Copy link
Copy Markdown
Contributor

This all looks good, I just have a few stylistic quibbles as follows:

  • molinit.c, line 271: the loop could and should be omitted, together with the commented-out lines within (also other commented-out lines in molinit()).

  • molinit.c, lines 290-295: this 'else' block (inherited from the previous code) could be made a small bit more conservative/safer by replacing it with something like

      } else if(g[id].t[0]<=part[ipart].temp[0]) {
        g[id].mol[i].partner[ipart].t_binlow=0;
        g[id].mol[i].partner[ipart].interp_coeff=0.0;
      } else {
        g[id].mol[i].partner[ipart].t_binlow=ntemp[ipart]-2;
        g[id].mol[i].partner[ipart].interp_coeff=1.0;
      }
    
  • stateq.c, line 128 (applies also to line 116 as inherited from the previous code): personally I would not use 't' here, since it makes one think of temperature. 'ti' perhaps, with 'li' at line 116.

  • stateq.c, lines 129-131: instead of 'trans_offset' defined as given, why not just something like

    int i = t*m[ispec].ntemp[ipart]+g[id].mol[ispec].partner[ipart].t_binlow;

Then the next 2 lines (actually a single line, wrapped) would become much more compact/readable as well as more conservatively expressed:

  double down = downrates[i] + g[id].mol[ispec].partner[ipart].interp_coeff*(downrates[i+1] - downrates[i]);

@tlunttil
Copy link
Copy Markdown
Contributor Author

tlunttil commented Jul 1, 2016

Yep, I forgot to remove the commented-out old parts. I'll do that, and your other suggested changes, and push a new version probably later today.

@allegroLeiden allegroLeiden merged commit e14af90 into lime-rt:master Jul 7, 2016
allegroLeiden pushed a commit to allegroLeiden/lime that referenced this pull request Jul 8, 2016
This was a difficult merge because PR lime-rt#135 by @tlunttil dealt with matters overlapping the present PR. I have modified the @tlunttil code
as follows:

  - His new double** 'down' attribute of struct molData stores the same data as the double* attribute 'colld' of struct cpData which I
introduced in the present PR. I have renamed 'colld' as 'down' but kept it as part of cpData.

  - The code where the new @tlunttil fields of struct rates are set has been moved from molini.c:molinit() to the appropriate place,
namely grid.c:calcGridCollRates(). The name of this function is no longer quite accurate, but I don't think that matters.
@tlunttil tlunttil deleted the coll_coeffs branch October 29, 2016 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants