Make mosaicking faster/memory efficient through healpix grid + Make it compatible to reduce truncated data#41
Make mosaicking faster/memory efficient through healpix grid + Make it compatible to reduce truncated data#41AkashA98 wants to merge 3 commits intoparsotat:develfrom
Conversation
… mosaicking, realtime analysis
|
@AkashA98 while I am reviewing these improvements, can you change the PR such that the changes be merged into the devel branch instead of main? |
There was a problem hiding this comment.
what is the purpose of this file? Is it used in the code that is included in the PR?
There was a problem hiding this comment.
if this is the old version of the code, this whole directory can be removed.
There was a problem hiding this comment.
I can do it, if you can confirm that this is just the old version of the code
| Returns: | ||
| """ | ||
|
|
||
| heatools.ftcoco( |
There was a problem hiding this comment.
I would like BatAnalysis to use astropy tools as much as possible and only use heasoftpy where it is actually needed.
| } | ||
|
|
||
|
|
||
| def convert_met_to_utc(times, invert=False): |
There was a problem hiding this comment.
This function doesnt include the effect of the clock offset, which is why the batlib functions that deal with MET/UTC end up calling the swiftbat package since that is taken care of there.
| else: | ||
| self.all_logs["batcelldetect"].append(batcelldetect_log) | ||
|
|
||
| def ftcopy(self, infile: str, outfile: str): |
There was a problem hiding this comment.
here is another example of a ftools script that we can try to convert to pure python to reduce dependencies on heasoftpy
| return new_tab | ||
|
|
||
|
|
||
| def post_process_catalogs(files, outfile=None, mode="merge"): |
There was a problem hiding this comment.
This should be converted to be astropy based
| self.__dict__.update(content) | ||
| print("Load successful.") | ||
|
|
||
| def _save(self, file): |
There was a problem hiding this comment.
do we want this to be _save? or save? ie should it be private or not? In line with BatSurvey it seems like it should be public
| ) | ||
|
|
||
|
|
||
| class MosaicProjections: |
There was a problem hiding this comment.
should this inherit from reprojectSurveys class? I have the MosaicBatSurvey class which inherits from the BatSurvey class since they share many properties (except for one being a mosaic) so I would naively expect the same here
| shutil.rmtree(self._local_pfile_dir) | ||
| return out | ||
|
|
||
| def load_default_params(self): |
There was a problem hiding this comment.
default parameters can be obtained eg:
BatAnalysis/batanalysis/bat_dph.py
Line 818 in 6f668fe
| recalc=False, | ||
| persistent_source_sep=10 * u.arcmin, | ||
| snr=5, | ||
| plot_dest_dir=None, |
There was a problem hiding this comment.
to keep with the structure of the rest of the code, plotting shouldn't be something that gets set in the __init__ method there should be separate methods that get called to explicitly do the plotting
2 participants
There are two main changes that are addressed by this PR: