-
-
Notifications
You must be signed in to change notification settings - Fork 409
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
csvy_model #936
csvy_model #936
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very good - there are a couple of things that need to be discussed and done - but otherwise - great work!!
|
||
csvy_model: csvy_full.csvy | ||
|
||
model: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how does that work? what takes precedent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How the model is loaded is handled by tardis/simulation/base.py in the from_config() class method. Currently, if the config has the 'csvy_model' keyword, then that is how the model is loaded. I think this should actually be handled in the schema, where the user is only allowed to define either 'model' or 'csvy_model'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
raise error if both are defined.
HomologousDensity | ||
|
||
""" | ||
if hasattr(csvy_model_config, 'velocity'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also horrible written - also not from you 😉
config.csvy_model) | ||
csvy_model_config, csvy_model_data = load_csvy(csvy_model_fname) | ||
base_dir = os.path.abspath(os.path.dirname(__file__)) | ||
schema_dir = os.path.join(base_dir, '..', 'io', 'schemas') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that should also not live here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validation should be done in load_csvy.
can you rebase to the master branch - because there are some changes from youssef in there. |
author Youssef15015 <[email protected]> 1560786513 +0200 committer Marc Williamson <[email protected]> 1562260267 -0600 parent 8d90018 author Youssef15015 <[email protected]> 1560786513 +0200 committer Marc Williamson <[email protected]> 1562260231 -0600 parent 8d90018 author Youssef15015 <[email protected]> 1560786513 +0200 committer Marc Williamson <[email protected]> 1562259570 -0600 Correct index script spelling (tardis-sn#940) Deploying docuemntation using Azure Update documentation of build Change repo label back to main Only 1 VM Remove travis yaml file Remove travis variable Changed documentation html Deleted unnecessary lines Remove sphinx_docs directory Update Build Documentation Pipeline Git pull exception, Deploy shell licensce Specify branch pull before build Update deploy_docs shell for branch Documentation and debugging [skip ci] Update licence and repo variable for pull Proof read documentation Prepare for pr Correct spelling mistake [skip ci] Co-authored-by: Jaladh Singhal <[email protected]> Update CI documendation Update documentation Update ssh-key documentation update azure documentation Clean pr Update documenation Rephrase documentation update documentation Co-authored-by: Jaladh Singhal <[email protected]> Update documentation Remove PR Trigger Set prs to not trigger in line 6 of the documentation build yaml file for azure pipelines Revert "Remove PR Trigger" Update build documentation script Added csvy_model property to base schema. Added load csvy_model using parser and validate. Added from_csvy class method. Added model_time_0 property Implemented density and velocity. Passes basic testing. Added test uniform abundances Added csvy csv abundance parser Implemented abundances. Radial1DModel can load a csvy file. Added tardis.model.tests data/ directory Implemented csvy_full_model test. Passes. Implemented csvy model tests. Passes all tests. Implemented t_radiative for csvy model Added t_radiative test Added option to use Rad1DMod.from_csvy Working on making full simulation run Fixed v_inner/outer boundary setting to match start/stop velocity. Identified bug in jblues update. Radial1DModel tests pass Changed long atom_file path names minor cleaning parametrized fixtures Fixed Isotope time_0, Implemented trad and w v_boundary slicing. Cleaned PR Added model_isotope_time_0 Changed model_time_0 to model_density_time_0 Isotope fix passes all pytests, tardis runs Fixed PEP 8 compliance errors Added documentation notebook for isotope class extension from Pandas DataFrame. Reverted trad and dilution factor 'fix' because tests fail. Added load csvy_model using parser and validate. Implemented density and velocity. Passes basic testing. Added test uniform abundances Implemented csvy model tests. Passes all tests. Working on making full simulation run Fixed v_inner/outer boundary setting to match start/stop velocity. Changed long atom_file path names minor cleaning parametrized fixtures Cleaned PR Added model_isotope_time_0 Changed model_time_0 to model_density_time_0 Isotope fix passes all pytests, tardis runs Fixed PEP 8 compliance errors Added documentation notebook for isotope class extension from Pandas DataFrame. Reverted trad and dilution factor 'fix' because tests fail. Made old v boundary index obsolete Rewrote v_boundary index properties. All tests pass. Incorporated init_trad_bug PR changes
No description provided.