Skip to content

Conversation

@jeffriley
Copy link
Collaborator

@jeffriley jeffriley commented Aug 9, 2025

(i) Deprecated option -use-mass-loss in favour of --mass-loss-prescription:

Instead of using --use-mass-loss or --use-mass-loss true to enable mass loss, then specifying the mass loss prescription to be used with --mass-loss-prescription, mass loss can be enabled using --mass-loss-prescription with any valid prescription (that is not zero), and disabled with --mass-loss-prescription zero instead of use-mass-loss false.

Note: users might see a few odd error message if they use --use-mass-loss and --mass-loss-prescription in the same command. Errors they may see are:

"Commandline Options error: option '--mass-loss-prescription' cannot be specified more than once"
"Commandline Options error: the required argument for option '--mass-loss-prescription' is missing"

Both are because the commandline parsing code in COMPAS replaces deprecated options before handing the commandline over to boost to parse for options, so we may have duplicated --mass-loss-prescription, and when we do that we may leave it without a parameter (because -use-mass-loss doesn't need "true" (though it can have it).

I could spend some time and energy (not insignificant) preventing those errors, but I don't think they'll cause too much confusion, and should be rarely seen.

(ii) Added compiler flag "-Wno-vla-cxx-extension" to "CXXFLAGS" in Makefile to suppress compiler extension warning

(iii) Fixed online docs for omissions in v03.22.02:
- fixed description for "--initial-mass-function" in "program-options-list-defaults.rst", and
- changed "--initial-mass-power" to "--initial-mass-function-power" in "program-options-list-defaults.rst"

@jeffriley jeffriley requested a review from ilyamandel August 9, 2025 07:10
@jeffriley jeffriley self-assigned this Aug 9, 2025
@jeffriley jeffriley added enhancement New feature or request severity_minor This bug is not very severe urgency_low This issue is not urgent labels Aug 9, 2025
@jeffriley jeffriley linked an issue Aug 9, 2025 that may be closed by this pull request
@jeffriley jeffriley marked this pull request as draft August 9, 2025 07:27
@jeffriley jeffriley marked this pull request as ready for review August 9, 2025 07:31
Comment in changelog.h was accidentally copied from an earlier PR
Copy link
Collaborator

@ilyamandel ilyamandel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks, @jeffriley !

I removed a repeated line from an earlier PR from changelog.h .

One optional suggestion: I think removing the use-mass-loss option should lead to an updated yaml.h and default yaml file. Yes, I realise the option is only deprecated, so nothing would go wrong with the old yaml file -- but I think of the yaml file as an "advertisement" of the available options, and if we don't want users to use deprecated options any more, we shouldn't advertise them.

@jeffriley
Copy link
Collaborator Author

I agree re yaml file @ilyamandel - completely slipped my mind. I'll push a new yaml file a bit later.

@ilyamandel
Copy link
Collaborator

P.S. One more thing: please remove --use-mass-loss from the list at the bottom of program-options-list-defaults.rst

@jeffriley jeffriley merged commit 52c8d75 into dev Aug 10, 2025
2 checks passed
@jeffriley jeffriley deleted the minor_fixes branch August 10, 2025 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request severity_minor This bug is not very severe urgency_low This issue is not urgent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Do we need --mass-loss-prescription NONE (now ZERO)?

3 participants