Energy Task preventing over/under-flow #352
Conversation
There was a problem hiding this comment.
Hi @Aske-Rosted,
Thanks for this contribution! I have made a few "inline" suggestions that you can consider using. Please also consider setting up pre-commit hooks, see our contributing guide as currently the automated black and flake8 checks are failing. Let me know if it would be beneficial to talk this over at some point. :)
|
@asogaard @Aske-Rosted Just wanted to add here that I also don't use the original class for the same reason. Perhaps we should consider if it should remain? |
Co-authored-by: Andreas Søgaard <andreas.sogaard@gmail.com>
Co-authored-by: Andreas Søgaard <andreas.sogaard@gmail.com>
There was a problem hiding this comment.
Hi @Aske-Rosted,
I have added one suggestion based on the automated mypy check. Another point: Perhaps we should remove the exist EnergyReconstruction -> EnergyReconstructionWithPower and instead just call the Task you're proposing EnergyReconstruction, also based on @RasmusOrsoe's point?
Co-authored-by: Andreas Søgaard <andreas.sogaard@gmail.com>
…n graphnet/src/graphnet/models/task/reconstruction.py
…t/models/task/reconstruction.py
|
Psst, @Aske-Rosted ... you're free to merge this anytime you want. :) |
|
Not sure how to, thought it would happen automatically when approved... |
Energy Task preventing over/under-flow
This task should be useful if experiencing over/underflow errors leading to NaN's in the training process.