MESAHub / mesa

Modules for Experiments in Stellar Astrophysics
http://mesastar.org
GNU Lesser General Public License v2.1
142 stars 38 forks source link

binary eval_mdot_edd.f90 cgrav is not set before use #8

Closed rjfarmer closed 4 years ago

rjfarmer commented 4 years ago
#0  0x150dbdcb327f in ???
#1  0x40a080 in __binary_mdot_MOD_eval_mdot_edd
    at ../private/binary_mdot.f90:450
#2  0x40a609 in __binary_mdot_MOD_adjust_mdots
    at ../private/binary_mdot.f90:493
#3  0x4101cb in __run_binary_support_MOD_do_run1_binary
    at ../private/run_binary_support.f90:421
#4  0x406be3 in __binary_lib_MOD_run1_binary
    at ../public/binary_lib.f90:72
#5  0x406858 in __run_binary_MOD_do_run_binary
    at ../../../../binary/job/run_binary.f:7
#6  0x406874 in binary_run
    at ../src/binary_run.f:4
#7  0x4068b5 in main
    at ../src/binary_run.f:2

line 449:

mdot_edd = 4d0*pi*b% s_donor% cgrav(1)*b% m(b% a_i)&
                  /(clight*0.2d0*(1d0+b% s_donor% surface_h1)*mdot_edd_eta)

b% s_donor% cgrav(1) has not been set yet. Looks like its the same issue as in r12596. That case we just switched to using standard_cgrav but this case i think we need to make sure cgrav is set earlier in the call sequence (during do_evolve_step_part1)

jschwab commented 4 years ago

Maybe better to just use standard_cgrav and document that limitation. I don't imagine this is a common application. If we do fix it, doesn't it seems more appropriate to have this be b% s_accretor% cgrav(1)? @orlox can make the call.

orlox commented 4 years ago

I think this should just be standard_cgrav, and document the limitation. This is mostly applied for point mass accretors in which case s_accretor does not even exist.

On Tue, Jan 28, 2020 at 10:59 PM Josiah Schwab notifications@github.com wrote:

Maybe better to just use standard_cgrav and document that limitation. I don't imagine this is a common application. If we do fix it, doesn't it seems more appropriate to have this be b% s_accretor% cgrav(1)? @orlox https://github.com/orlox can make the call.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/MESAHub/MESA/issues/8?email_source=notifications&email_token=AAEDFIC4UKB2PUIPJK77YCLRACTELA5CNFSM4KIHDIQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKFCB2Q#issuecomment-579477738, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEDFIFMXBWK4B3UXESHPL3RACTELANCNFSM4KIHDIQQ .

-- Pablo Marchant Campos M.Sc on Astrophysics, Universidad Católica de Chile PhD on Astrophysics, Argelander-Institut für Astronomie, Universität Bonn

orlox commented 4 years ago

So I've decided to simply purge the use of s% cgrav in binary. It opens a whole can of worms of not clearly defined things. Mdot edd is just an example, but for instance what cgrav should we use to define the orbital angular momentum? The change is up in revision 12647.

I'd say we just put an absolute warning when documenting the option, indicating that the binary module is unaffected by changes in cgrav. Another more in-your-face way to do that would be to have binary detect if the user is using other_cgrav and spit out a warning every step about this caveat (with an option to turn off the warning).

jschwab commented 4 years ago

Great, thanks. Documenting that other_cgrav only affects the individual stellar model seems like a good approach to me.

jschwab commented 4 years ago

Added a note to this effect in other_cgrav.f90 in r12663.