Rewritten TS model with segmented laser geometry #219

Mateasek · 2020-03-17T11:13:40Z

I decided to do new PR, because at the end there were many changes:

laser geometry is now segmented cylinder as suggested by @mattngc. This propagated into handling of information between plasma, laser node, laser model and scattering model. This was neccessary to simplify handling of notifications, deletions and etc.
- laser model and spectrum hold no plasma and laser reference. Their methods return requested properties (e.g. laser power) in laser node frame.
- Laser material now obtains all the information from plasma, laser spectrum and laser model needed by the scattering model.
- Scattering model works in the frame of reference of the laser node. Method for scattered spectrum calcullation must now get all the neccessary plasma and laser variables (e.g. Te, ne, directions, etc.)
laser models were rewritten and new added (uniform, bivariate and trivariate normal spatial distributions, Gaussian beam model). Lasers are now described by pulse energy and pulse temporal lengh, this is transformed to spatial power densities using given distributions.
laser spectrum power spectral density was corrected.

Unittests checking the following were added:

laser node
- initialisation
- propagation of notifications between laser model, laser spectrum, scattering model, laser geometry and laser material
- position of the laser segments compared to the expected one
laser spectrum
- initialisation and changes
- normalisation of models
laser models
- initialisation
- polarisation direction
- correct values of power for spatial points
- integration of laser power over pulse spatial extent is compared to expected pulse energy
scattering model
- scaterred spectrum traced by a ray is compared to expected values calculated by semi-analytical approach.

I hope that this time I minimised PEP8 incompatibilitity. If this state of the model is acceptable and won't be changed much, I will move to writting docstrings, documentation and examples:

Docstrings
Documentation
Examples

Segmented cylinder as laser geometry rewritten laser model to correct power units new laser model laser material obtains all properties needed for scattering calculation laser_spectrum and laser_model independent of plasma or laser added tests for laser

CnlPepper · 2020-03-22T01:07:34Z

I'm going to propose we merge this into a feature branch first, so we can make a few tweaks before the merge into development.

CnlPepper · 2020-03-22T01:10:05Z

This PR is looking a lot better and a lot cleaner. I see there are potentially some areas of improvement in the notification/change handling. I should be able to address that after merge if necessary, provided I'm sure of the original intentions. There are a few things I'd tweak. The material does seem a little over complicated. Some of the properties on the material should really be on the laser etc... if you compare the laser material with the beam material, that is really as complex as it should be.

Laser material simplified. Most of the calculations moved to the scattering model. Caches of plasma and laser transforms ScatteringModel got reference to the plasma and laser_model to provide values for the scattering calculations ScatteringModel changed to LaserEmissionModel Model manager handling emission models added to laser Propagation of changes and notifications changed to be more like the beam model.

Mateasek · 2020-05-23T18:09:29Z

I have simplified the material to be closer to the beam material as suggested by @CnlPepper. The code was moved to the scattering emission calculation. I have inspired myself in the beam model and simplified the notifications.

What should be the next step? Should I remove the draft flag so it is merged to the feature branch?

CnlPepper · 2020-05-31T17:12:32Z

I think we should get it in a feature branch and do any final iterations there. I'm going to do a quick check over now.

CnlPepper · 2020-05-31T19:05:42Z

I've added some comments to the existing review.

Segmented cylinder as laser geometry rewritten laser model to correct power units new laser model laser material obtains all properties needed for scattering calculation laser_spectrum and laser_model independent of plasma or laser added tests for laser

Laser material simplified. Most of the calculations moved to the scattering model. Caches of plasma and laser transforms ScatteringModel got reference to the plasma and laser_model to provide values for the scattering calculations ScatteringModel changed to LaserEmissionModel Model manager handling emission models added to laser Propagation of changes and notifications changed to be more like the beam model.

CnlPepper · 2020-11-03T13:22:51Z

Hey @Mateasek I see you marked this as ready for review, however there are some outstanding queries above.

cherab/core/laser/material.pyx

cherab/core/laser/models/math_functions.pyx

cherab/core/laser/node.pyx

cherab/core/laser/scattering.pxd

cherab/core/laser/models/model.pyx

cherab/core/laser/scattering.pyx

cherab/core/laser/node.pyx

…ek/cherab_core into feature/ThomsonScattering

move files model* to profile*

vsnever

Hi @Mateasek,

This is a very important new feature. As far as I know, the Thomson scattering synthetic diagnostic is in great demand at ITER. Thanks a lot for this!

The comments are many but all the issues are minor.
Also, could you please update the changelog?

cherab/core/laser/__init__.py

cherab/core/laser/laserspectrum.pxd

cherab/core/laser/laserspectrum.pyx

cherab/core/laser/profile.pyx

cherab/core/laser/laserspectrum.pyx

demos/laser/model_seldenmatoba.py

demos/laser/thomson_scattering.py

There is currently no need to keep laser reference n LaserProfile. This was a leftover from version, where some laser properties were in Laser

minor ones

In LaserSpectrum class

from laser._plasma to laser.plasma

Rebuilding segment primitives was separated from assigning materials to segment primitives. Rebuilding primitives is now called only in needed cases. Added configure_geometry to laser_profile notifier Fixed missing addition of _plasma_changed to plasma notifier Changing integrator changes only integrators within materials

Mateasek · 2022-09-07T14:18:48Z

Thank you very much @vsnever for doing this very thorough review and for suggesting the improvements and spotting problems. In case of the minor suggestions I applied the suggestions and marked them as resolved. There are few regarding the notifications and rebuilding of the geometry and materials, which I left opened because the applied changes should be checked again. I would like to ask you to see if it is what you had in mind, thanks.

vsnever

Thanks for fixing all the issues. Now everything looks fine, except for a couple of leftover attributes.

cherab/core/laser/profile.pxd

cherab/core/model/laser/model.pxd

vsnever

Thanks @Mateasek, I'm happy with this PR.

Mateasek added 2 commits March 17, 2020 10:50

Multiple changes:

4da5231

Segmented cylinder as laser geometry rewritten laser model to correct power units new laser model laser material obtains all properties needed for scattering calculation laser_spectrum and laser_model independent of plasma or laser added tests for laser

remove empty method

871aced

CnlPepper changed the base branch from development to feature/laser March 22, 2020 01:06

mattngc added the enhancement label Mar 30, 2020

mattngc linked an issue Mar 30, 2020 that may be closed by this pull request

Add a basic Thomson Scattering model #97

Closed

mattngc assigned Mateasek Mar 30, 2020

mattngc requested a review from CnlPepper March 30, 2020 18:18

Mateasek added 5 commits May 14, 2020 23:16

Merge branch 'development' into TS/development

6277b57

Merge branch 'development' into TS/development

7d80da6

Optimise code, remove redundant imports and weak refs

18cc572

Merge branch 'TS/development' into feature/ThomsonScattering

9b63b37

Mateasek marked this pull request as ready for review June 1, 2020 13:47

Mateasek added 4 commits September 1, 2020 20:13

Multiple changes:

d0ef093

Segmented cylinder as laser geometry rewritten laser model to correct power units new laser model laser material obtains all properties needed for scattering calculation laser_spectrum and laser_model independent of plasma or laser added tests for laser

remove empty method

10f26d5

Optimise code, remove redundant imports and weak refs

7631e96

CnlPepper requested changes Nov 5, 2020

View reviewed changes

Mateasek added 3 commits January 15, 2021 12:31

Merge branch 'feature/ThomsonScattering' of https://github.com/Mateas…

c7e044b

…ek/cherab_core into feature/ThomsonScattering

Fix scattering constants and laser units

963264e

fix imports, rename constants

b1a6af6

Mateasek mentioned this pull request Feb 22, 2021

Class for managing lists holding same types #272

Open

Mateasek added 2 commits February 22, 2021 16:27

fix more imports

3d13a0f

rename LaserModel to LaserProfile

e00365f

move files model* to profile*

vsnever requested changes Aug 16, 2022

View reviewed changes

Mateasek added 14 commits September 5, 2022 16:00

Change imports

b1fea94

Fix typo

1e1f3ef

Remove photon related quantities from LaserSpectrum

1674110

Remove laser reference from LaserProfile

9cd8dab

There is currently no need to keep laser reference n LaserProfile. This was a leftover from version, where some laser properties were in Laser

Remove reversed logic

106fd25

Remove more inverse logic

f90a533

Add minor codereview suggestions and fixes

6455fc7

Apply multiple code review suggestions

04dbaf9

minor ones

Replace leading _ in attributes

c0b2d07

In LaserSpectrum class

Change accesing private attribute

c4f2dae

from laser._plasma to laser.plasma

Changed accessing properties to attributes

72fee42

Add tests to check if integrators are changed

8baf130

Fix wrong docstring

e351baf

Update changelog

cc29924

vsnever reviewed Sep 7, 2022

View reviewed changes

cherab/core/laser/profile.pxd Outdated Show resolved Hide resolved

cherab/core/model/laser/model.pxd Outdated Show resolved Hide resolved

Remove redundant attributes

4f7dbbb

vsnever approved these changes Sep 7, 2022

View reviewed changes

Mateasek changed the base branch from feature/laser to development September 13, 2022 10:21

Mateasek requested a review from CnlPepper October 31, 2022 08:17

Mateasek changed the base branch from development to feature/laser October 31, 2022 08:19

Mateasek changed the base branch from feature/laser to development October 31, 2022 08:19

Merge branch 'development' into feature/ThomsonScattering

29267ab

Mateasek force-pushed the feature/ThomsonScattering branch from 914dbc2 to 29267ab Compare October 31, 2022 10:46

Mateasek and others added 2 commits October 31, 2022 11:51

Remove empty line

ab1277c

Merge branch 'development' into feature/ThomsonScattering

90929ea

jacklovell merged commit a7caca2 into cherab:development Dec 22, 2022

vsnever mentioned this pull request Dec 1, 2023

Add a basic Thomson Scattering model #97

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewritten TS model with segmented laser geometry #219

Rewritten TS model with segmented laser geometry #219

Mateasek commented Mar 17, 2020 •

edited

Loading

CnlPepper commented Mar 22, 2020 •

edited

Loading

CnlPepper commented Mar 22, 2020 •

edited

Loading

Mateasek commented May 23, 2020

CnlPepper commented May 31, 2020

CnlPepper commented May 31, 2020

CnlPepper commented Nov 3, 2020

vsnever left a comment

Mateasek commented Sep 7, 2022

vsnever left a comment

vsnever left a comment

Rewritten TS model with segmented laser geometry #219

Rewritten TS model with segmented laser geometry #219

Conversation

Mateasek commented Mar 17, 2020 • edited Loading

CnlPepper commented Mar 22, 2020 • edited Loading

CnlPepper commented Mar 22, 2020 • edited Loading

Mateasek commented May 23, 2020

CnlPepper commented May 31, 2020

CnlPepper commented May 31, 2020

CnlPepper commented Nov 3, 2020

vsnever left a comment

Choose a reason for hiding this comment

Mateasek commented Sep 7, 2022

vsnever left a comment

Choose a reason for hiding this comment

vsnever left a comment

Choose a reason for hiding this comment

Mateasek commented Mar 17, 2020 •

edited

Loading

CnlPepper commented Mar 22, 2020 •

edited

Loading

CnlPepper commented Mar 22, 2020 •

edited

Loading