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

New plots to analyse Nitrogen abundance #265

Merged
merged 10 commits into from
Jan 12, 2024

Conversation

correac
Copy link
Contributor

@correac correac commented Nov 5, 2023

New plots for log10(C/O) vs 12+log10(O/H) and log10(N/O) vs 12+log10(O/H). The goal of these relations is to test the primary and secondary production channels of Nitrogen nucleosynthesis.

Copy link
Collaborator

@robjmcgibbon robjmcgibbon left a comment

Choose a reason for hiding this comment

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

We need to wait until https://github.com/SWIFTSIM/velociraptor-comparison-data/pull/179 is merged so you can update the submodule link

colibre/registration.py Outdated Show resolved Hide resolved
colibre/registration.py Outdated Show resolved Hide resolved
@robjmcgibbon
Copy link
Collaborator

Could you please also upload some images of the new plots

@MatthieuSchaller
Copy link
Member

Is this still [WIP] ?

@correac
Copy link
Contributor Author

correac commented Nov 29, 2023

@MatthieuSchaller I'm checking consistency in everything now since I have four merge requests (SOAP, velociraptor-python, pipeline-configs and observational_dataset). Between today and tomorrow I will remade a few plots and recheck all changes to remove the WIP.

@MatthieuSchaller
Copy link
Member

Any news on this? Still WIP?

@correac correac changed the title [WiP] New plots to analyse Nitrogen abundance New plots to analyse Nitrogen abundance Jan 10, 2024
@correac
Copy link
Contributor Author

correac commented Jan 10, 2024

New plots for log10(C/O) vs 12+log10(O/H) and log10(N/O) vs 12+log10(O/H). The new figures are shown below. @MatthieuSchaller @robjmcgibbon I reviewed the changes and I am happy, this PR can be merged if there are no outstanding issues.
image

image

image

@correac
Copy link
Contributor Author

correac commented Jan 10, 2024

I am adding four new plots to this PR. I hope that's ok! The new plots correspond to the stellar abundance plots with some variations: (1) [N/O] vs [O/H], (2) [N/O] vs [Fe/H], (3) [C/O] vs [O/H], (4) [C/O] vs [Fe/H]. Find the new figures below.
image
image
image
image

Copy link
Collaborator

@robjmcgibbon robjmcgibbon left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me, but I have a minor query about plot comments, and then a request to cut down some code

colibre/registration.py Outdated Show resolved Hide resolved
colibre/auto_plotter/metallicity.yml Outdated Show resolved Hide resolved
@robjmcgibbon
Copy link
Collaborator

Perfect, all looks good to me! Are you happy to merge https://github.com/SWIFTSIM/velociraptor-comparison-data/pull/179? We need to update the submodule link before we merge this PR

@correac
Copy link
Contributor Author

correac commented Jan 12, 2024

Yes! I am happy to merge https://github.com/SWIFTSIM/velociraptor-comparison-data/pull/179 . I reviewed it the other day and it looks good. Thank you Rob!!

@robjmcgibbon robjmcgibbon merged commit bc8949b into SWIFTSIM:master Jan 12, 2024
1 check passed
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.

3 participants