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

add the ability to reassemble live calculated TimeChunked quantities #47

Open
wants to merge 84 commits into
base: master
Choose a base branch
from

Conversation

mtremmel
Copy link
Contributor

@mtremmel mtremmel commented Apr 5, 2018

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.

Michael and others added 30 commits October 31, 2017 12:43
…ty to calculate inner SFHistory of galaxies. Change bins to 2000 (10 Myr).
… max_radius) and new way of calling halo centering
mtremmel and others added 26 commits September 8, 2018 12:33
# Conflicts (fixed):
#	tangos/properties/__init__.py
…pdate SF.py to include new halo argument to reassemble() in star formation rate histogram classes.
@mtremmel
Copy link
Contributor Author

mtremmel commented May 5, 2020

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())')
Copy link
Member

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"?>
Copy link
Member

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

Copy link
Contributor Author

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

@apontzen
Copy link
Member

apontzen commented May 5, 2020

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.

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.

2 participants