search

threeML / hawc_hal

HAWC Accelerated Likelihood - python-only framework for HAWC data analysis
BSD 3-Clause "New" or "Revised" License
11 stars 21 forks source link

Adding radial profiles to HAL #68

Closed torresramiro350 closed 2 years ago

torresramiro350 commented 2 years ago

I have implemented a working example of radial profiles in HAL. The function follows the same principle as in the 3ML HAWCLike.py plugin without the dependence on LiFF functions. The function get_excess_background picks out the active HealPix pixels at different radial bins and calculates the signal, background and model counts at each bin. The function get_radial_profile takes care of the 'ring' excess calculations. Lastly, the plot_radial_profiles takes care of the plotting. The arguments are the same as in the HAWCLike plugin. I've include a testing script to ensure it works accordingly.

xiaojieww commented 2 years ago

Also, I noticed that this branch is 17 commits behind threeML:master, could you update it first? @torresramiro350

torresramiro350 commented 2 years ago

the branch has been updated. let me know if you experience any other issues.

henrikef commented 2 years ago

@xiaojieww @torresramiro350 What's the status of this PR? Any remaining issues?

@torresramiro350 could you push some trivial change (even adding whitespace to the Readme should work) to trigger the automated tests?

xiaojieww commented 2 years ago

@torresramiro350 I apologize for the slow response. @henrikef Some tests with astromodels didn't succeed, should we worry about this? Not sure if this is related to recent updates?

henrikef commented 2 years ago

You will need to merge #71 first (and then merge the new master into this branch). I just merged the current master into #71 and restarted the tests.

xiaojieww commented 2 years ago

71

After the tests are done, I will merge the #71 first, then go back here.

xiaojieww commented 2 years ago

@torresramiro350 Can you merge the new master into your branch and then trigger the test? I just merged the #71 , it included propagating changes in astromodels interface to HAL.

torresramiro350 commented 2 years ago

@xiaojieww Done.

xiaojieww commented 2 years ago

It still has three checks that didn't pass the check. Seems they had the same checks failed before I merge #71, but these tests were passed in #71. Do you @henrikef and @torresramiro350 have any ideas?

henrikef commented 2 years ago

Well it looks like it goes into some infinite loop/wait cycle and the test gets canceled after some time. Can you reproduce the issue locally on a mac computer?

xiaojieww commented 2 years ago

Well it looks like it goes into some infinite loop/wait cycle and the test gets canceled after some time. Can you reproduce the issue locally on a mac computer?

Sorry I missed this comment. For the threeML and astromodels test without HAL installation, it passed on Mac. We can see this in #78 as well. All the tests passed on Mac but the CI test failed for mac.