Skip to content

Conversation

@andLaing
Copy link
Collaborator

Removes the use of the file electrons_40keV_z250_RWF.h5 in
the isidora tests and in the commandline tests for isidora
and irene.

This file was found to have badly written MC info in PR #693

@andLaing andLaing requested a review from paolafer March 30, 2020 15:56
@paolafer
Copy link
Collaborator

There are other places in the code where this file is used, maybe it would be safer to replace it everywhere, checking that the tests that use it don't rely on the specific content of the file. What do you think about it?

@andLaing
Copy link
Collaborator Author

Seems reasonable. The initial commits just replaced it where the test accessed the MC info. I'll do another set of commits removing it completely. Should I remove the file from the repository too?

@paolafer
Copy link
Collaborator

Yeah, I think we can remove it, once it is not used anymore.

@paolafer
Copy link
Collaborator

For instance, to me electrons_40keV_z25_RWF.h5 seems the most similar file we have: it has the same number of events (10), it's the same kind of particle (40-keV electrons), the only difference being the z initial position of the electrons.

Copy link
Collaborator

@paolafer paolafer left a comment

Choose a reason for hiding this comment

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

This PR eliminates a test file that had wrong MC true information. The tests that used it have been modified to use a different file with similar content and the file has been removed. All the tests succeed now, so I approve this PR.

@andLaing andLaing force-pushed the change_RWF_file_isidora branch from 536169c to ea20f84 Compare March 31, 2020 14:55
electrons_40keV_z250_RWF.h5 had badly written
MC info.

Tests now use electrons_40keV_z25_RWF.h5
which has no mistake in the MC info.

Change files_in in all configs which used the faulty
file to use the same replacement file

Remove electrons_40keV_z250_RWF.h5 from fixture
electron_RWF_file
@andLaing andLaing force-pushed the change_RWF_file_isidora branch from ea20f84 to 9d97a04 Compare March 31, 2020 15:04
@carmenromo carmenromo merged commit 3062929 into next-exp:master Mar 31, 2020
@andLaing andLaing deleted the change_RWF_file_isidora branch April 10, 2020 15:02
andLaing pushed a commit to andLaing/IC that referenced this pull request May 13, 2020
next-exp#713

[author: andLaing]

Removes the use of the file electrons_40keV_z250_RWF.h5 in
the isidora tests and in the commandline tests for isidora
and irene.

This file was found to have badly written MC info in PR next-exp#693

[reviewer: paolafer]

This PR eliminates a test file that had wrong MC true information. The
tests that used it have been modified to use a different file with
similar content and the file has been removed. All the tests succeed
now, so I approve this PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants