Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RedistributeGeometryDecorator: axial and spherical powerlaw #202

Merged
merged 9 commits into from
Nov 8, 2023

Conversation

arlauwer
Copy link
Contributor

@arlauwer arlauwer commented Nov 7, 2023

Description
Added an abstract RedistributeGeometryDecorator that redistributes density distributions by multiplying it by a weight function. Currently there are 2 concrete sub-classes implementing a spherical and axial power law weight function.

Motivation
This decorator allows for adding gradients to already implemented geometries, which would otherwise have no analytical inverse cumulative. Such as for example hyperboloids and paraboloids that are not constant.

Testing
Testing involved comparing:

  • spherical shell with power law p + a spherical decorator with power law q
  • shell geometry with a power law of p+q
    Additionally, analytical values were compared with SKIRT-generated values for density and images to validate the decorator's functionality.

Context
More RedistributeGeometryDecorator can be added if needed. There is also a spherical and axial exponential weight function, but these have not been included.

@petercamps
Copy link
Contributor

Good work. I introduced a few minor changes.

Most importantly, when a configurable property is "irrelevant" its value should never be used. This is why I added the extra tests in line 32 of AxPowerLawRedistributeGeometryDecorator.

Also, by convention, we add a . at the end of floating point constants such a 0. and 1. even though the compiler will almost certainly perform the conversion from integer to double at compile time.

@petercamps petercamps merged commit 8cd4722 into SKIRT:master Nov 8, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants