Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix XSX and update XDC source for DEFUSE #2

Merged
merged 8 commits into from
Aug 15, 2024
Merged

Conversation

samueljackson92
Copy link
Collaborator

This changes the soft x-ray source code to fix a bug with adding camera params. This also removes most of the XDC processing for now. We only capture IPref as this is required by DEFUSE.

@samueljackson92 samueljackson92 added the bug Something isn't working label Aug 12, 2024
@samueljackson92 samueljackson92 self-assigned this Aug 12, 2024
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_xsx_camera_params fails, believe it is because of line 304 or 310 (need to look into the logs). Changing the name maybe

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea. Will fix this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more for curiosity, will we be re-introducing the tensorising bits later on then?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. But we've currently done this quite crudely. Nathan and I have some notes from talking to charlie vincent about xdc. We can do this a bit better than the current implementation. But it should not be done here. Here I just want to get the one signal that is important for DEFUSE.

@samueljackson92
Copy link
Collaborator Author

@jameshod5 I think this is ready to be re-reviewed please!

@jameshod5
Copy link
Collaborator

Looks okay to me

@jameshod5 jameshod5 merged commit 5c3821d into main Aug 15, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants