Relativity #697
Relativity #697
Conversation
tardis/montecarlo/src/cmontecarlo.c
Outdated
| @@ -139,12 +139,55 @@ binary_search (const double *x, double x_insert, int64_t imin, | |||
| return ret_val; | |||
| } | |||
|
|
|||
| void | |||
| do_angle_aberration (rpacket_t *packet, const storage_model_t *storage) | |||
There was a problem hiding this comment.
Since this is only active during full_relativity I'd do the relativity check in this function instead of outside.
That way you can't execute the function by accident and the code would be cleaner.
The disadvantage is you'd have to look at the function implementation to know this is full_relativity only. But I think we can live with that.
There was a problem hiding this comment.
I think this is an excellent idea.
tardis/montecarlo/src/cmontecarlo.c
Outdated
| * @return CMF direction cosine | ||
| */ | ||
| double | ||
| inverse_angle_aberration (rpacket_t *packet, const storage_model_t *storage, double mu) |
There was a problem hiding this comment.
Same as above.
Maybe explicitly add inline directive?
There was a problem hiding this comment.
I am a bit worried about clarity here. It is not always obvious if mu is measured in the LF or in the CMF. I have suggested a number of name changes to improve the readability but we may also think about the possibility of introducing additional packet properties, specifically storing the state in the CMF/LF.
| return 1.0 - | ||
| rpacket_get_mu (packet) * rpacket_get_r (packet) * | ||
| storage->inverse_time_explosion * INVERSE_C; | ||
| double beta = rpacket_get_r (packet) * storage->inverse_time_explosion * INVERSE_C; |
There was a problem hiding this comment.
One could again think about introducing more clarity here:
doppler_factor_CMF_to_LF
doppler_factor_LF_to_CMF
If this naming scheme is considered too cumbersome, I'd at least advise to add an inline comment about the transformation direction!
|
|
||
| if (storage->full_relativity) | ||
| { | ||
| current_nu = current_nu * (1 + beta * current_mu) / sqrt(1 - beta * beta); |
There was a problem hiding this comment.
Couldn't you use the doppler_factor routines for this?
Codecov Report
@@ Coverage Diff @@
## master #697 +/- ##
==========================================
+ Coverage 66.23% 66.54% +0.31%
==========================================
Files 83 83
Lines 5917 5978 +61
==========================================
+ Hits 3919 3978 +59
- Misses 1998 2000 +2
Continue to review full report at Codecov.
|
Addresses TEP010: Consistent Treatment of relativistic effects.
This PR implements:
Still missing relativistic effects:
These missing effects are considered non-critical and insignificant.