Modifications to Maltsev prescription

[reinhold-willcox]

Modifications to the Maltsev remnant mass prescription:

• Added switch for different Maltsev modes: Optimistic, Balanced, and Pessimistic
• Added MatlsevFallback option to fix the fallback fraction when using Maltsev model
• Added fixed remnant masses for the different Maltsev outcomes, instead of the stochastic ones from MandelMuller
• Added lum and teff as attributes of the RLOFProperties typedef, which help determine where a star is in the HR diagram before MT.

[ilyamandel]

@reinhold-willcox -- see the complaint from th auto spell checker ("src/GiantBranch.cpp:1975: interally")

[ilyamandel]

GiantBranch.cpp:

if (utils::Compare(p_COCoreMass, MALTSEV2024_MMIN) < 0) {                                                           // NS formation regardless of metallicity and MT history
    m_SupernovaDetails.fallbackFraction = 0;
    remnantMass = 1.35; //CalculateRemnantNSMassMullerMandel(p_COCoreMass, p_HeCoreMass);

Do you really want all NS to be exactly 1.35 Msun? That wasn't what we initially agreed with Kiril, but OK. If so, please deleted the old code rather than just commenting it out.

======

The long comment here is completely out of place -- there's nothing about MT case in this check.
// TODO: rewrite this
// the only way this can happen is if someone added an MT_CASE
// and it isn't accounted for in this code. We should not default here, with or without a warning.
// We are here because DetermineMassTransferTypeAsDonor() returned an MT_CASE this code doesn't
// account for, and that should be flagged as an error and result in termination of the evolution
// of the star or binary.
// The correct fix for this is to add code for the missing MT_CASE or, if the missing MT_CASE is
// incorrect/superfluous, remove it from the possible MT_CASE values.

            THROW_ERROR(ERROR::UNKNOWN_MALTSEV_MODE);                                                                   // throw error

=====

        double mhe = m_SupernovaDetails.HeCoreMassAtCOFormation;
        double mns = 1.44;
        remnantMass = mns + (mhe - mns) *m_SupernovaDetails.fallbackFraction;
    }
    else {
        m_SupernovaDetails.fallbackFraction = 0;
        remnantMass = 1.40; // slightly lower mass NS - just to distinguish it...
        
        
I expect @jeffriley will complain about the case in mHe, mNS. ;)  Also, despite the comment that mNS is slightly lower, it's actually slightly higher than for lower-mass cores, which yield mNS of 1.35 Msol.  In fact, I wonder if 1.35, 1.40 and 1.44 (all used for NS masses in this prescription) should be defined as constants.

====

//fallbackfraction determined internally

fallback fraction should be two words

[ilyamandel]

I see new program options and new outputs. That means both documentation files need to be updated. Also, yaml.h should be updated and a new ../compas_python_utils/preprocessing/compasConfigDefault.yaml file should be pushed.

[ilyamandel]

rlofPropertiesToReset->teff1                       = m_Star1->Temperature();
rlofPropertiesToReset->teff2                       = m_Star2->Temperature();
rlofPropertiesToReset->lum1                        = m_Star1->Luminosity();
rlofPropertiesToReset->lum2                        = m_Star2->Luminosity();

I would personally prefer luminosity1, temperature1, etc., but leave this to @jeffriley . :)

No other comments from me!

[reinhold-willcox]

I think I've addressed all the points above, this should now be ready for a full review.

[jeffriley]

@reinhold-willcox I will look at this later today. Two things off the top of my head:

(a) build checks are not running on github at the moment (only precommit checks - don't know why, @avivajpeyi is going to look at it). Hvae you successfully built COMPAS locally with your branch, with no warnings or errors?

(b) is there really nothing that users (not developers) need to know about these changes? i.e. nothing to put in "What's New"?

[jeffriley]

Looks good. Just a couple of requests:

in GiantBranch::CalculateRemnantMassByMaltsev2024(), these:

double    log10_1          = 0;
double    log10_1_div_10   = -1;    
double    log10_1_div_50   = -1.69897;

should be constexpr double (unless you want them to be modifiable...)

And for this:

double    M1, M2, M3;

Could you add a comment to describe what M1, M2, and M3 are?

And, finally, this is an enhancement, so the version should move from v03.23.01 to v03.24.00 (not v03.23.02)

[reinhold-willcox]

Thanks @jeffriley. All points addressed.

The src compiles, with only the following warning. Can it be ignored?

g++ -std=c++17 -g -fnon-call-exceptions   -I/include -I/include -I/usr/include/hdf5/serial -I. -c yaml.cpp
utils.cpp: In function ‘std::string utils::GetGSLVersion()’:
utils.cpp:1839:48: warning: ignoring attributes on template argument ‘int (*)(FILE*)’ [-Wignored-attributes]
 1839 |         std::unique_ptr<FILE, decltype(&pclose)> pipe(popen("gsl-config --version", "r"), pclose);          // open pipe for command
      |                                                ^
g++ -std=c++17 -g -fnon-call-exceptions   -I/include -I/include -I/usr/include/hdf5/serial -I. -c vector3d.cpp

[jeffriley]

The src compiles, with only the following warning. Can it be ignored?

Oy, I am tired of compiler incompatibilities... I don't see that warning when I compile. I'm using WSL 2 with Ubuntu 22.04, and gnu c++ 11.4.0

You can ignore it - I'll change that code in utils::GetGSLVersion() to be a bit tighter in what pointer types it uses, and the warning will go away.

[jeffriley]

Thanks @reinhold-willcox . Just one final thing: could you note the addition of the two new options (--maltsev-fallback and --maltsev-mode) in What's New?

I'll approve now, and you can merge after you've updated What's New

[reinhold-willcox]

Ah it still needs @ilyamandel to approve the requested changes before I can merge.

[ilyamandel]

The program options documentation should have the new options added to the list at the bottom of the page.

Which variant of the MALTSEV remnant mass prescription.

Not a complete sentence.

Compactness-peak BHs formed CO-mass progenitors

formed from ?

[ilyamandel]

Approving, but please address minor comments above before merging

[reinhold-willcox]

@ilyamandel I've fixed both. Though fwiw, most of the first sentence descriptions of the other program options are also not complete sentences. The way I changed it is still not a full sentence, but it reads a little clearer.