Feat/add chandra images #1411
Open
Feat/add chandra images #1411
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…eing thrown by the execute_cmd was just a good old fashioned Python error (not from the product) and so looking for the ' is associated source' string to split on and find the source repr was throwing another error! For issue #1264 (indirectly)
…m chandra_image_expmap, and also added the other two output product types to the sources_types list that is returned
…iles loaded into multiple output products. For issue #1264
…ifferent products output from one command a little better. For issue #1264
… types as well as lists of product types
…me format whether it is for one product type or many.
…be, with multiple entries. For issue #1264
… For issue #1264 (indirectly)
…ate into being passed into the execute_cmd function properly - trying a fix.
…mage does for Chandra) which just continues (XGA will produce a ratemap itself) - but this should change eventually I feel. Relevant to this is issue #1251
…erent types produced by a single function! It should also provide returns that are compatible with the existing states of sas_call and esass_call (so I don't have to change those right now). It can now return a list of products instead of a single product, so will need to adapt ciao_call to make that work. In aid of issue #1264
…already be able to deal with multiple products in the returned 'results' dictionary (containing the set up product instances). It is concerning that I don't remember why that would be, as I am now worried that I might have broken something for another telescope! For issue #1264
…n array as I was allowing before
…, etc. that Ray sets up in the chandra_image_expmap function to the stupidly formatted names that fluximage comes out with. For issue #1264
…tion run work. For issue #1264
… it was wiping it each iteration. Hopefully fixing this will make image generation work for sources with multiple Chandra ObsIDs. For issue #1264
…age_expmap up one level of indent, I think they were being set too earlier. For issue #1264
…) bit applied to the queue type works fine with a single ObsID and a single or multiple expected output types, but it doesn't work with multiple ObsIDs because it still makes it completely flat rather than having one type entry (be it a string or an array) for each command. Adding some diagnostics to figure out the best solution
…single and multiple obs, and with single and multiple expected output products.
Signed-off-by: rz-wang <rzwang@umich.edu>
#1265) Signed-off-by: rz-wang <rzwang@umich.edu>
Signed-off-by: rz-wang <rzwang@umich.edu>
Signed-off-by: rz-wang <rzwang@umich.edu>
Signed-off-by: rz-wang <rzwang@umich.edu>
Signed-off-by: rz-wang <rzwang@umich.edu>
…/addChandraImages
…d parameter, ensuring that specextraction follows core_excised during iteration and defaults to core_excised in the final run. I duplicated the original function since the LT pipeline supports multiple missions, and I want to avoid altering the existing one until sufficient testing has been completed (#1358). Signed-off-by: rz-wang <rzwang@umich.edu>
…d a new vikhlinin scaling relation to test the core_included case (core_excised=False). (#1373) Signed-off-by: rz-wang <rzwang@umich.edu>
…"core_excised" property of the ScalingRelation class (#1358). Signed-off-by: rz-wang <rzwang@umich.edu>
…1358) Signed-off-by: rz-wang <rzwang@umich.edu>
Signed-off-by: rz-wang <rzwang@umich.edu>
Signed-off-by: rz-wang <rzwang@umich.edu>
Signed-off-by: rz-wang <rzwang@umich.edu>
Signed-off-by: rz-wang <rzwang@umich.edu>
Signed-off-by: rz-wang <rzwang@umich.edu>
Signed-off-by: rz-wang <rzwang@umich.edu>
…ne the size of the cluster core; set by default of 0.15xR_vir; also added the same parameter to the Chandra R2500 scaling relation (#1373) Signed-off-by: rz-wang <rzwang@umich.edu>
rz-wang
added
the
telescope specific
DavidT3
requested changes
Aug 5, 2025
There was a problem hiding this comment.
Hi Ray,
This is really nice work (and a lot of it, took me a while to review!)
I've left quite a few comments but there is nothing major, and some of the comments are more to remind me that I'll have to be careful when I merge the PR.
I would read all of the comments before you start working on anything, and feel free to reply to them so we can chat about it more, or to push back if you don't like whatever idea I've had.
Cheers,
David
| @@ -234,6 +243,18 @@ def luminosity_temperature_pipeline(sample_data: pd.DataFrame, start_aperture: Q | |||
| raise ValueError("The max_iter value ({mai}) is less than or equal to the min_iter value " | |||
| "({mii}).".format(mai=max_iter, mii=min_iter)) | |||
|
|
|||
| # Check whether the specified core_excised value matches that of the scaling relation | |||
There was a problem hiding this comment.
If we did just use the information from the input scaling relation to define whether we'll do core-excised or not, we could get rid of all of this
There was a problem hiding this comment.
Why is this part related to 'core_excised'? Isn’t it meant to let the user define the maximum and minimum number of iterations they want (and just make sure the max_iter > min_iter)?
…I also marked those conversations as resolved, as I don't think they need to remain open under PR (#1413) Signed-off-by: rz-wang <rzwang@umich.edu>
Signed-off-by: rz-wang <rzwang@umich.edu>
Signed-off-by: rz-wang <rzwang@umich.edu>
…define the overdensity or a fixed apeerture. Changed the 'core_radius' parameter to 'core_radius_fraction'. (#1413) Signed-off-by: rz-wang <rzwang@umich.edu>
Signed-off-by: rz-wang <rzwang@umich.edu>
Signed-off-by: rz-wang <rzwang@umich.edu>
Signed-off-by: rz-wang <rzwang@umich.edu>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've made quite a few updates over the past few months. I should probably push smaller PRs more frequently instead of merging everything into a large one like this. That said, here are the main changes — most of them involve new Chandra-related features:
I haven't tested the float precision of the radii yet, as the updated version is in a different branch. However, this should be the first thing to test after this PR is merged.