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

Documentation for using/getting a new CARSUS atom_data file for STARDIS #226

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

RyanGroneck
Copy link
Contributor

📝 Description

Type: 📝 documentation

This is a first draft of the STARDIS documentation to show someone how to get a relevant atomic_data file from CARSUS and how to use that new data in a STARDIS simulation. Keep an eye out to make sure I didn't use wrong terminology or explain something incorrectly as I'm still new to this :)

📌 Resources

Examples, notebooks, and links to useful references.

🚦 Testing

How did you test these changes?

  • Testing pipeline
  • Other method (describe)
  • My changes can't be tested (explain why)

☑️ Checklist

  • I requested two reviewers for this pull request
  • I updated the documentation according to my changes
  • I built the documentation by applying the build_docs label

Note: If you are not allowed to perform any of these actions, ping (@) a contributor.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@RyanGroneck
Copy link
Contributor Author

in the added part for vald_linelist, I was unsure what shortlist: <boolean> is used for, so that will need to be expanded upon (presumably)

@@ -0,0 +1,102 @@
{
Copy link
Contributor

@jvshields jvshields Nov 4, 2024

Choose a reason for hiding this comment

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

Make sure to use a spellchecker on your documentation. "quntities" is mispelled.

I'm not sure I'd make the distinction between elements and atoms, but also CARSUS can include molecular information and does not particularly include information on the quantities of chemicals in the model, so I'd just say explicitly "on the properties or atoms and molecules needed by STARDIS."

Also, I'd remove the part about kurucz being recommended. Rather, I'd say that, for a detailed and accurate stellar spectrum we highly recommend looking into using a VALD linelist bundled into your atomic data.


Reply via ReviewNB

@@ -0,0 +1,102 @@
{
Copy link
Contributor

@jvshields jvshields Nov 4, 2024

Choose a reason for hiding this comment

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

"STARDUS" typo

Also, "strictly necessarily" is probably supposed to be "necessary." That said, I wouldn't say "necessary" anyway, since you don't technically need a VALD linelist or molecular data from Barklem & Collet. I'd just say that they're recommended.


Reply via ReviewNB

@@ -0,0 +1,102 @@
{
Copy link
Contributor

@jvshields jvshields Nov 4, 2024

Choose a reason for hiding this comment

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

Try to be a bit more intentional with which font styles you're using when. It's a bit odd how this text is small and bold, and different from other text.


Reply via ReviewNB

@@ -0,0 +1,102 @@
{
Copy link
Contributor

@jvshields jvshields Nov 4, 2024

Choose a reason for hiding this comment

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

Weird command and then period at the end of this line.


Reply via ReviewNB

@@ -0,0 +1,102 @@
{
Copy link
Contributor

@jvshields jvshields Nov 4, 2024

Choose a reason for hiding this comment

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

The shortlist flag is relevant to the format of the linelist. VALD can output lists in either "long" form or "short" form, so you need to tell STARDIS which one it is.


Reply via ReviewNB

@jvshields
Copy link
Contributor

It might be easier to look at my comments on ReviewNB. It's a bit harder to tell which lines I'm talking about on github.

@jvshields jvshields added the documentation Improvements or additions to documentation label Nov 4, 2024
@jvshields
Copy link
Contributor

You will also need to add this file to the index.rst file in docs to have it show up when the documentation gets built. Take a look at that and how you need to include it there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants