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

Set "eos" argument for Garching models #164

Closed
soso128 opened this issue Dec 14, 2021 · 5 comments · Fixed by #171
Closed

Set "eos" argument for Garching models #164

soso128 opened this issue Dec 14, 2021 · 5 comments · Fixed by #171
Labels
bug Something isn't working

Comments

@soso128
Copy link
Contributor

soso128 commented Dec 14, 2021

Hi,

I noticed there is a keyword argument in the constructor of the GarchingArchiveModel class to set the EOS. Is there a way to set the value of this argument when calling the generate_time_series function of snowglobes.py? I didn't see any **kwargs option.

Best regards,
Sonia

@JostMigenda
Copy link
Member

Hi Sonia! Thus far, all data files corresponding to the _GarchingArchiveModel class (i.e. Bollig_2016, Tamborra_2014, Walk_2018 and Walk_2019) use the LS220 EOS, so unless I’m missing something, it shouldn’t be necessary to manually set that argument? But you’re right; if we add model files in that format with a different EOS at some point, we’ll need to make some changes.

@soso128
Copy link
Contributor Author

soso128 commented Dec 14, 2021

Hi Jost,
Yes indeed I was wondering about how to change the EOS if we want to try a different model in this class. But I guess I can just add a **kwargs in generate_time_series and use it when calling the constructor.

@JostMigenda
Copy link
Member

Ah, I see—yes, that sounds like a good solution! Once you’ve done that, could you please create a PR so everyone can benefit from this?

@JostMigenda JostMigenda added the bug Something isn't working label Dec 14, 2021
@JostMigenda
Copy link
Member

After thinking about it some more, I realized that this also affects the OConnor_2013 model, which has a keyword argument for the progenitor mass. Right now, generate_time_series() cannot pass that argument, so users can only use the default mass with snewpy.snowglobes. So this issue also affects users who don’t use custom models and I’ll mark it as a bug.

@soso128
Copy link
Contributor Author

soso128 commented Jan 12, 2022

Hi, sorry for my late reply. I have opened a pull request with a possible fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants