Open jeffriley opened 2 days ago
I mean, the frequency really is infinite when the birth spin period is zero (non rotating). It's not a physically very sensible choice, but it is correct. I don't think setting them to 0 is the right thing to do (that corresponds to an infinite spin period, which is not consistent).
The spin down rate should just return 0 if the spin period is 0 (it can't spin down any more than non-spinning).
I wouldn't really call this a major-bug either.
As an aside, why do we need to store m_PulsarDetails.spinFrequency in memory if we can trivially compute it as a division?
A couple of things:
the frequency really is infinite when the birth spin period is zero (non rotating)
How can the frequency with which a body is rotating be infinite if it isn't rotating? Surely if something isn't rotating, the frequency with which it is rotating is zero. If anything should be infinite shouldn't it be the period (the time it takes for one full rotation)?
I wouldn't really call this a major-bug either.
The problem is that we divide-by-zero. Not only shouldn't we do that, if I turn fp-error handling on (not available yet on macOS), I don't get past this divide-by-zero... so fp-error handling is rendered almost useless...
Right, spin period = 0 is a bit of a special case. As spin periods get lower and lower, the frequency gets higher and higher. Technically, a spin period of zero, if taken as a limit, should correspond to infinitely fast rotation. Thinking further (and correcting a comment I made above), we should in fact store spin frequencies, not spin periods, and work with these only. A spin frequency of zero is unambiguously a sign of no rotation (as frequencies decrease, the body spins slower, so this is a natural limit of infinitely slow rotation).
Right, but here it's not the limit - a birth spin period of 0.0 here is meant to indicate it's not spinning (as you noted above ('...when the birth spin period is zero (non rotating)')).
we should in fact store spin frequencies, not spin periods, and work with these only. A spin frequency of zero is unambiguously a sign of no rotation
I agree with that - let's do that :-) And if we need to calculate the period from frequency we should nt divide by zero - if the star isn't spinning we could avoid a lot of the calculation in the NS code, couldn't we?
(as you noted above ('...when the birth spin period is zero (non rotating)')).
That was @SimonStevenson , not me [just giving credit :) , I agree with your solution]
Running COMPAS with:
results in
inf
inNS::CalculateAndSetPulsarParameters
in the following statement:because
CalculateBirthSpinPeriod()
in this statement:returns 0.0 because the default
PULSAR_BIRTH_SPIN_PERIOD_DISTRIBUTION
isZERO
The naive solution is to change the statement:
to:
so that
m_PulsarDetails.spinFrequency
is set to 0.0 ifCalculateBirthSpinPeriod()
returns 0.0@yuzhesong , @SimonStevenson, can you confirm that the suggestion above is the best solution?
A related problem occurs in
NS::CalculateSpinDownRate()
, when parameterp_Omega
is passed as 0.0. This statement:results in
inf
The naive solution is to add the following sanity check as the first line of the function:
so we just return a spin down rate of 0.0 if the spin freqency passed in
p_Omega
is 0.0@yuzhesong , @SimonStevenson, can you confirm that the suggestion above is the best solution?
A further related problem becomes evident when the above two problems are addressed: the following statement in
NS::SpinDownIsolatedPulsar()
results ininf
:which can naively be fixed by replacing the statement with:
But maybe there's a better solution - can we avoid calling some of these functions if the spin frequency is 0.0?
@yuzhesong, @SimonStevenson Is there a better fix overall when spin frequency is 0.0?
To Reproduce Run COMPAS with:
Expected behavior Production code does not cause
inf
**Versioning
OS: Ubuntu 22.04 COMPAS v03.07.01