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

Error on quasi universal fit parameters #398

Open
lshetusuleiman opened this issue Oct 4, 2024 · 4 comments
Open

Error on quasi universal fit parameters #398

lshetusuleiman opened this issue Oct 4, 2024 · 4 comments
Assignees
Labels
bug Something isn't working question Further information is requested

Comments

@lshetusuleiman
Copy link

lshetusuleiman commented Oct 4, 2024

Dear developpers,
In Spacetime.py, a quasi universal relation on the dimensionless mass quadrupole uses fits established in Algendy & Morsink 2014.
However, it seems that the hard coded value of beta1 is not the one reported in Table 1 (page 4) of Algendy & Morsink 2014, see copy of this table:
image

It should be (according to the paper) 0.4454 and it is coded 0.4554.

return temp + 0.4554 * 4.0 * self.epsilon * self.zeta / 3.0

This is in lines ~ 190 of Spacetime.py. Can you confirm that this is a typo ?

Also in the same function, does someone knows why there is no minus sign in front of the 0.11, value for the a2 parameter ?

temp = self.epsilon * 0.11 / (self.zeta * self.zeta)

And finally, I am a bit puzzled by this line

temp -= self.a * self.a / (self.r_g * self.r_g)

that I cannot seem to find in the paper of Algendy & Morsink. Would you happend to know what it does ?

Kind regards,
Lami Suleiman.

@sguillot sguillot added bug Something isn't working question Further information is requested labels Oct 4, 2024
@thjsal
Copy link
Contributor

thjsal commented Oct 4, 2024

Well noticed! I think those seem indeed typos. Luckily, I think we are usually not using this function q() (to be confirmed though). Anyway, makes sense to fix it (or remove if not needed at all). And I am not sure about the puzzling line. Should look more into that.

@thjsal
Copy link
Contributor

thjsal commented Oct 4, 2024

The actual oblateness relations we use are linked in this issue:

#359

@thjsal
Copy link
Contributor

thjsal commented Oct 4, 2024

Okay, I checked that this function is never called in the GitHub fast-example and thus very likely not in any of our other analyses so far.

@sguillot sguillot added the hackweek2024 Issues to be fixed during the 2024 Hack Week label Dec 2, 2024
@sguillot
Copy link
Contributor

sguillot commented Dec 2, 2024

I think it should be fixed anyway - we don't use it, but other users might.

The fix to line 200 and 202 are trivial, but I do not understand where line 201 of spacetime.py comes from. Any ideas?

@drannawatts drannawatts removed the hackweek2024 Issues to be fixed during the 2024 Hack Week label Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants