foxsi / foxsi-smex

Tools for scientists to investigate the capabilities of FOXSI
MIT License
2 stars 8 forks source link

Adding the FOXSI PSF for any position to the IDL repository #21

Closed aringlis closed 8 years ago

aringlis commented 8 years ago

Work in progress - not to be merged yet. This is the IDL version of the functionality added in #20

ehsteve commented 8 years ago

Fixed my PR so you should update yours I think.

LinErinG commented 8 years ago

@aringlis @ehsteve I'm missing psf_parameters.txt, so the code doesn't run for me. Should I already have that somewhere?

ayshih commented 8 years ago

@LinErinG @aringlis I'm currently fixing up this PR, so you might want to wait until I'm done. But to answer your question, it's from Steve's PR.

ayshih commented 8 years ago

Andy needs to merge the PR I just submitted to his branch. Here's an example (with oversampling set to 3):

screen shot 2015-11-11 at 3 30 29 am

I have not yet cross-checked this against the Python side, the IDL side looks to be doing the right thing.

aringlis commented 8 years ago

I just merged in @ayshih's updates.

aringlis commented 8 years ago

Removed the WIP label but it would be good for someone else to test this out.

aringlis commented 8 years ago

Oh I see, you have to explicitly be in the foxsi-smex/idl directory. I reverted the change.

ayshih commented 8 years ago

:+1:

LinErinG commented 8 years ago

@aringlis foxsi_get_output_image_cube.pro is breaking for me at line 347 with the new PSF. (I double-checked; it works ok in the current main version.) Looking at it, I don't see obviously why it's breaking, but it does look like foxsi_get_output_image_cube.pro doesn't have all the same fixes to it that foxsi_get_output_2d_image.pro has. Should the same changes be implemented in that version, which does the same thing as 2d_image except repeats it several times for different energies?

ehsteve commented 8 years ago

Merging this because Natasha is asking about it. We can do a separate bug fix for @LinErinG issue.

aringlis commented 8 years ago

@LinErinG my mistake - I neglected the image_cube file. I'll post a fix for this.

ayshih commented 8 years ago

@aringlis sorry, was already working on it, and just posted the PR (#24)

aringlis commented 8 years ago

@ayshih - awesome, thanks!

LinErinG commented 8 years ago

@aringlis @ehsteve @ayshih Reviving an old, finished pull request. This function doesn't seem to work for me. I get the same PSF no matter what off-axis position I put into it. @aringlis can you verify that this function returned significantly different PSFs for different off-axis positions at the time you did the PR? Does it work when you try it now?

ayshih commented 8 years ago

@LinErinG Regardless of whether or not the code is working as intended, the current PSF parameters file has no dependence on off-axis angle (as of commit 9782cb96f2f8a315d97d46124f61bfe6fb8990e9), so you would see no variation with off-axis angle without changing that file.

LinErinG commented 8 years ago

@aringlis @ehsteve @ayshih Ok, I'll add this as an issue, since one can only find that out by digging in the code to see what it calls. Does a parameters file for off-axis angles exist in anybody's branch? We need it for a FOXSI rocket application, and there's time pressure.

aringlis commented 8 years ago

@LinErinG @ayshih @ehsteve - I have an old psf_parameters.txt file that seems to have information as a function of off-axis angle. But, I dimly recall that we intentionally removed this functionality from the code, as we were not convinced we were doing it properly - does that ring any bells?

ayshih commented 8 years ago

That commit I referenced (9782cb9) has that old parameters file, but @ehsteve will need to refresh everyone's memories on the precise reasons for zeroing out the dependence.