Fix stellar age#138
Conversation
…i,ox are stored in the same positions in the array as it is done for lllustris
|
tests are failing with missing argument 'gamma'. Can we fix this? then I do the review. |
There was a problem hiding this comment.
Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- notebooks/nohup.out: Language not supported
Comments suppressed due to low confidence (1)
rubix/galaxy/input_handler/pynbody.py:112
- [nitpick] Using a hardcoded index (4) for placing OxMassFrac may reduce clarity; consider defining a constant or using a named index for improved readability.
metals[:, 4] = ox_data["OxMassFrac"]
| setattr(component, "coords", coords) | ||
| setattr(component, "velocity", velocities) | ||
|
|
||
| return rubixdata" |
There was a problem hiding this comment.
Extraneous string literal after the return statement appears to be an unintended docstring terminator. Please remove the spurious quotes to ensure proper function behavior.
| return rubixdata" | |
| return rubixdata |
There was a problem hiding this comment.
and this is still to be fixed, right?
| vel_rot = apply_init_rotation(velocities, R) | ||
| pos_final = apply_rotation(pos_rot, alpha, beta, gamma) | ||
| vel_final = apply_rotation(vel_rot, alpha, beta, gamma) | ||
| if key == "ILlustrisTNG": |
There was a problem hiding this comment.
The string 'ILlustrisTNG' appears to be a misspelling of 'IllustrisTNG'. Correct the string to ensure consistent behavior.
| if key == "ILlustrisTNG": | |
| if key == "IllustrisTNG": |
There was a problem hiding this comment.
this file should not be in the repo.
Let's remove
|
@anschaible @ufuk-cakir also we should probably clean this one up. |
for more information, see https://pre-commit.ci
TobiBu
left a comment
TobiBu
left a comment
There was a problem hiding this comment.
Sorry, there is one more thing I found...
| self.logger.info("Metals assigned to gas particles.") | ||
| self.logger.info("Metals shape is: %s", self.data["gas"]["metals"].shape) | ||
|
|
||
| self.data["stars"]["age"] = 14.14019767 * u.Gyr - self.data["stars"]["age"] |
There was a problem hiding this comment.
why do we have 14.14 Gyr? shouldn't the universe be 13.8 Gyr?
There was a problem hiding this comment.
probably that should be connected to the cosmology we specify in the yaml file...
see here:
Line 234 in dbb4487
| hdr1["EXTNAME"] = "DATA" | ||
| hdr1["OBJECT"] = object_name | ||
| hdr1["BUNIT"] = "erg/(s*cm^2*A)" # flux unit per Angstrom | ||
| hdr1["BUNIT"] = "10**-20 erg/(s*cm^2*A)" # flux unit per Angstrom |
| """ | ||
| logger.info("Rotating galaxy for simulation: " + config["simulation"]["name"]) | ||
| # Rotate gas | ||
| if "gas" in config["data"]["args"]["particle_type"]: |
There was a problem hiding this comment.
this part looks like it can be refactored, but only minor comment
| vel_rot = apply_init_rotation(velocities, R) | ||
| pos_final = apply_rotation(pos_rot, alpha, beta, gamma) | ||
| vel_final = apply_rotation(vel_rot, alpha, beta, gamma) | ||
| elif key == "NIHAO": |
There was a problem hiding this comment.
maybe add comment here why we distinguish between illustris and nihao
The merge-base changed after approval.
-Fix rotation for nihao
-Fix gas metals for nihao
-Fix stellar age for nihao