Skip to content

Cs cell interpolation#116

Closed
JopHendrikx wants to merge 3 commits intomainfrom
cs-cell-interpolation
Closed

Cs cell interpolation#116
JopHendrikx wants to merge 3 commits intomainfrom
cs-cell-interpolation

Conversation

@JopHendrikx
Copy link
Collaborator

@JopHendrikx JopHendrikx commented Apr 11, 2024

Calculate cell cross sections on the fly for inelastic/ionization/attachment operators, together with corresponding output parameters. Removes double interpolation, such that linear interpolation errors are as minimized as possible. Also with this the numerical threshold is found on the grid cells instead of at the nodes.

Operator distribution is now defined from target cell towards source cell.

@codecov
Copy link

codecov bot commented Apr 16, 2024

The author of this PR, JopHendrikx, is not an activated member of this organization on Codecov.
Please activate this user on Codecov to display this PR comment.
Coverage data is still being uploaded to Codecov.io for purposes of overall coverage calculations.
Please don't hesitate to email us at support@codecov.io with any questions.

@daanboer
Copy link
Member

@JopHendrikx it seems that commits 8d54025 and 923ac8b should not be part of this PR. Is that correct?

@janvdijk
Copy link
Collaborator

@JopHendrikx, I was going through the open pull requests, then stumbled on this one. I noticed 1) a question of @daanboer about some patches that are (possibly) mistakenly part of this request, and 2) merge conflicts. Will you need help with any of this?

Apart from the technical issues: we should think carefully about this change. This will result in deviations w.r.t. to the MATLAB code, so it would be better to first agree with @IST-Lisbon and @AntonioTejero about the desired operation. (A convincing before vs. after testcase will help, of course.)

@daanboer
Copy link
Member

@janvdijk, @JopHendrikx the commit history has been cleaned up and merge conflicts are resolved (rebased onto main). In theory this could be merged (perhaps with some additional testing), but I agree that it is better to first discuss with @IST-Lisbon, and @AntonioTejero.

@daanboer daanboer self-assigned this Jun 12, 2024
@janvdijk
Copy link
Collaborator

janvdijk commented Jun 12, 2024

@janvdijk, @JopHendrikx the commit history has been cleaned up and merge conflicts are resolved (rebased onto main). In theory this could be merged (perhaps with some additional testing), but I agree that it is better to first discuss with @IST-Lisbon, and @AntonioTejero.

@daanboer, after #168 gets merged, I plan to implement the parts of #116 that are described in #169 first. That will significantly reduce the size of #116 and avoids the controversial parts of it. It will just: 1) calculate the cell-values of totalCrossSection() by interpolating from that field after it has been updated, and 2) use that member instead of doing the calculation in many places. (BTW, #116 should be modified to capture const references to the totalCellCrossSection(), at present copies are made, as in (ElectronKinetics.cpp line 375):

   Vector cellTotalCrossSection(mixture.collision_data().totalCellCrossSection());

This should be

   const Vector& cellTotalCrossSection(mixture.collision_data().totalCellCrossSection());

... the same in many other places. I will add that change to the cs-cell-interpolation branch.

UPDATE: @daanboer, I just looked, but that is too messy. I will just wait for your review of #168 (and hopefully its merger), make these change afterwards on a separate branch, then merge those back to #116.

@daanboer daanboer force-pushed the cs-cell-interpolation branch from 5790f0e to dbd151b Compare June 12, 2024 21:26
janvdijk pushed a commit that referenced this pull request Jun 13, 2024
 * Grid.h: add free function interpolateNodalToCell
 * EedfCollision.*: use that, store the totalCellCrossSection
   and provide an accessor.
 * ElectronKinetics.cpp: use totalCellCrossSection(), do not
   repeat this task multiple times.

See issue #169. Note that this does the non-controversial parts
of #116.
daanboer pushed a commit that referenced this pull request Jun 17, 2024
* Store totalCellCrossSection, centralize interpolation.

 * Grid.h: add free function interpolateNodalToCell
 * EedfCollision.*: use that, store the totalCellCrossSection
   and provide an accessor.
 * ElectronKinetics.cpp: use totalCellCrossSection(), do not
   repeat this task multiple times.

See issue #169. Note that this does the non-controversial parts
of #116.

* Add a test for interpolateNodalToCell.

Test that grid.getNodes() is correctly interpolated up to
grid.getCells(), up to essentially the machine accuracy.

* Add documentation for interpolateNodalToCell.

* Integrals.h -> GridOps.h, add cell-derivation

File Integrals.h has been renamed into GridOps.h. It now hosts various
grid-related operations: interpolation, differentiation and integration.
The inline interpolation function has been moved there from Grid.h.
Tests have been added for the new cell->cell differentiation function
cellDerivative, which has been extracted from ElectronKinetics (function
calculateFirstAnisotropy). A test has been added (that u'=1).

* Minor comment fixes and additions.

* Remove unused variable 'n'.

---------

Co-authored-by: Jan van Dijk <jan@epgmod.phys.tue.nl>
@daanboer daanboer force-pushed the cs-cell-interpolation branch from dbd151b to b25b7a2 Compare June 17, 2024 12:02
@daanboer daanboer force-pushed the cs-cell-interpolation branch from b25b7a2 to a8c96b3 Compare March 11, 2025 11:45
@daanboer
Copy link
Member

daanboer commented Oct 1, 2025

I will close this for now, as this might be fixed implicitly by merging #232, or similar changes.

@daanboer daanboer closed this Oct 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants