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

Forked repository for disc C/O vs M_star work #2

Closed
wants to merge 10 commits into from
Closed

Forked repository for disc C/O vs M_star work #2

wants to merge 10 commits into from

Conversation

jymah
Copy link
Contributor

@jymah jymah commented Dec 6, 2023

contains all the changes (gas.py, bertmigration.py) and the config+job files needed to reproduce the results of Mah et al. (2023).

@AaronDavidSchneider
Copy link
Owner

Hi @jymah,

awesome! Thanks for uploading these files. I think, I can not merge the PR, since some of the changes (e.g., the ones to gas.py) would overwrite what the code is supposed to do by default.

I will think about it, but maybe you could incorporate the changes into a subclass so that the default behaviour is not affected, or alternatively describe the needed changes to the files somewhere and leave the code unaffected.

@jymah
Copy link
Contributor Author

jymah commented Dec 7, 2023

Hello @AaronDavidSchneider

I see the problem. I would suggest we take the 2nd option which is to leave the main code untouched. This is because the modifications for a linear gas accretion rate is an ad-hoc thing to make things simple for the paper. In the future, one should ideally use the more "complete" prescription to compute the gas accretion rate.

I will revert the changes, or maybe I'll remove this fork and start over :p

@jymah jymah closed this by deleting the head repository Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants