-
Notifications
You must be signed in to change notification settings - Fork 70
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
A range of fixes: using core radii for CE survival, fixes to #1255, #1258, #1265 #1291
Conversation
Build Successful! You can find a link to the downloadable artifact below.
|
@ilyamandel I just pushed some changes that add to your fix. There is also a question for you at the end of |
Thanks, @jeffriley -- I hadn't meant to ask you to do my work for me, but I appreciate it. ;-) |
No worries @ilyamandel - as soon as I read that we were still reporting HeWD I knew why, and the fix was easy (and really should already have been in - continuing evolution after a stellar merger was an oversight). The switchlog issue. When I coded the output all I wanted to indicate was: (a) the starting stellar type for each star In the specific case of the example in issue #1286, that worked until you put the switches to MR in :-) By switching the stars to MR after the stellar merger, you've actually continued the evolution and changed the "ending" stellar type. But both stars becoming MR is not really physical, is it? You're just doing that so you get an entry in the switchlog? If we evolved merger products, something different would happen and we'd still get a switch, right? Just not to both MR. What I'm getting at is that I think you're doing something unphysical just so you get an entry in the switchlog, and if that's true then trying to manipulate the output to account for that is a bit of a kludge - maybe we should figure out the right way (or a better way) of getting an entry in the switchlog upon stellar merger? Maybe we should always evolve past a stellar merger and switch the stars to what they should become upon merger - but that would mean we don't output what they were just prior to the merger... Do we want to change the output so in the case of mergers we print the stellar types of the stars just prior to the merger as well as the final stellar types? Whatever we do, users have to be confident that what we output is consistent (i.e. we shouldn't change the meaning of what's output just for stellar mergers - whatever we do should be additional info). Hopefully that makes sense. |
online-docs/pages/User guide/Program options/program-options-list-defaults.rst
Outdated
Show resolved
Hide resolved
@jeffriley -- I take your point, and will think some more about what is a sensible "short-hand" output to the screen. For now, I think what's in this PR is the best solution until we actually implement the evolution of merger products (other than MS merger products). Ready for your review! |
@ilyamandel what about we add a new column to the BSE switchlog file ("isMerger"), and create a function that can be called from anywhere that logs a merger record to the BSE switchlog file? It would list the from and to stellar types as the same (because no switch), and they would be whatever the stellar types were at the time of the call to log the record. The "isMerger" field would be "false" for ordinary switch logs, and "true" for the merger logs. The function to log the merger would be a no-op if the switchlog is not enabled. Thoughts? I could do this in the next day or so. |
I like that, @jeffriley ! We can take out lines 3088-3090 of the new BaseBinaryStar.cpp file -- either now or in your next PR, your call. P.S. I am going offline now, good night. ;-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ilyamandel
No description provided.