Closed TjarkMiener closed 3 years ago
This PR would be ready for a code review! Thank you, @nietootein @aribrill!
Thanks @LucaRomanato for reporting a bug occur in the subfolder structure. 6733ed0
should fix this!
I am in the process of testing the PR on shevek and will have more comments soon. One issue I came across so far is the installation. I followed the installation instructions in the README. Are they still up to date, e.g. regarding installing DL1DH with pip? I had to run the following commands after setup to get things working:
source activate ctlearn
conda install -c cta-observatory ctapipe-extra
pip uninstall numpy
pip install numpy
Thanks for the comments @aribrill! I resolved most of them. Regarding the installation, can you specify the ctapipe and dl1dh version you used please? ctapipe-extra should not be required though. The multi-stereo mode is still present in the DL1DH version, which are used for this PR.
Also thanks for fixing the number of examples of DL1DH. I will have a look into it now.
I used DL1DH v0.8.2, which requires ctapipe 0.8.0.
On Tue, Mar 2, 2021 at 9:54 AM Tjark Miener notifications@github.com wrote:
Thanks for the comments @aribrill https://github.com/aribrill! I resolved most of them. Regarding the installation, can you specify the ctapipe and dl1dh version you used please? ctapipe-extra should not be required though.
Also thanks for fixing the number of examples of DL1DH. I will have a look into it now.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ctlearn-project/ctlearn/pull/132#issuecomment-788965481, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEWJLHIWMHUX3XEZGAMPTR3TBT3ZBANCNFSM4PEPLJWA .
The versions are correct. I think pip is messing up the dependencies. The correct versions of DL1DH and ctapipe should be installed automatically by conda, when the environment is created. There is no need to git clone the DL1DH repo and installed DL1DH from source. The latest commits should fix the installation and I updated the README. Can you please check, if the correct versions with the correct dependencies are installed on shevek
? I also added a commit to make ctlearn a command line tool to avoid pointing to the run_model script. This should work: $ ctlearn myconfig.yml
Actually, I have a question. I replicated one of the benchmark models (arr_lowcut_TRN) and would like to plot the predictions for diffuse gammas. I have a predictions file in h5 format. I see that none of the plotting scripts have been updated to be compatible with this new format, however. Are there new scripts that replicate the old functionality or at least some instructions for what to do with this file?
I also see that ctaplot has been added as a dependency, but I can't find where it's used. Is that related in some way?
Thanks for the fixes! Everything worked fine for the compatibility PR. I uploaded the visualisation notebook. Anything missing?
Thanks for updating the plotting scripts!
This PR includes three major features:
Energy and arrival direction regression (Impact parameter and shower maximum opitonal)
Residual blocks
Attention mechanisms
Some minor features:
Minor restructuring of the run_model.py. Separating the data loading (setting up the DL1DH: inputfn() etc.) and the output handling from the main script.
This PR requires DL1DH PR 85.