-
Notifications
You must be signed in to change notification settings - Fork 13
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
add the ability to reassemble live calculated TimeChunked quantities #47
base: master
Are you sure you want to change the base?
Conversation
…ty to calculate inner SFHistory of galaxies. Change bins to 2000 (10 Myr).
…ent property module
…ll particles and not just gas.
… max_radius) and new way of calling halo centering
…to conserve memory
# Conflicts (fixed): # tangos/properties/__init__.py
…ive_reassemble
…pdate SF.py to include new halo argument to reassemble() in star formation rate histogram classes.
Hi @apontzen just wanted to ping you here. I've implemented tests as well as cleaned up the code considerably. I realize this has been drawn out in time, so I understand if it takes a while to parse things. Most recently, I have made the branch up-to-date with master and ensured that all tests pass by fixing a bug in the previous version which made normal reassembled properties fail. |
no relevant arguments to give, but any argument taken by the live calculation will work like normal). | ||
|
||
``` | ||
halo.calculate('reassemble(SpecSFR_histogram())') |
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.
Why is reassemble
required?
@@ -0,0 +1,6 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> |
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.
We probably don't want this file in the repo
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.
yup sorry that's my bad I can remove that
Thanks Michael - I've got some comments in there which I think should be looked at. They are mainly old comments but I'm not sure they were ever addressed? Specifically, we seem to be coupling modules together in a way they weren't previously, which makes me nervous. |
Changes to LiveProperty and the reassemble functions to allow the reassembly of a live calculated TimeChunked property. Generally, this would be some function that manipulates already existing time chunked data such as SFR or SMBH accretion histories.
Also, in order to make things a bit more general, config.py does not automatically attempt to load nbodyshop properties but now only looks to the property modules defined by the environment variable. This change comes as I've had issues with this when attempting to load multiple external property modules. This also seems better as the code is now public and shouldn't be assuming anything about property modules the user wishes to load.