-
Notifications
You must be signed in to change notification settings - Fork 93
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
Graph Definition #558
Graph Definition #558
Changes from all commits
ae4bada
08cd6b4
238324c
ea52b8d
f7e93b2
8a1cace
ccf9d32
b4b1023
c18e5e1
99d9b51
9b9434d
8984a01
d5c8ade
6d15019
fa30515
4a994bc
0a21d75
56474ca
50e6afe
469d396
ce248de
118b580
639bf3a
a22a794
3e01dcd
fed3900
c417ce8
6627d62
365c55b
1de129a
53a1861
378ea8e
f22c986
5110a91
e7c9a11
5dc1298
c510ba0
dc1485d
41da3ab
fac6794
5924d2f
f7a3aca
2d0b5cf
a2e0508
e72678d
1fb67cc
0e26587
a7216e4
dcd00c5
cec8c48
cab8c74
9eb948a
5f1c5fe
10e2eda
af94365
df68aa3
76140f2
f20f2ed
f174cf3
3fb96c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,26 +1,34 @@ | ||
path: $GRAPHNET/data/tests/sqlite/oscNext_genie_level7_v02/oscNext_genie_level7_v02_first_5_frames.db | ||
pulsemaps: | ||
- SRTInIcePulses | ||
features: | ||
- dom_x | ||
- dom_y | ||
- dom_z | ||
- dom_time | ||
- charge | ||
- rde | ||
- pmt_area | ||
truth: | ||
- energy | ||
- position_x | ||
- position_y | ||
- position_z | ||
- azimuth | ||
- zenith | ||
- pid | ||
- elasticity | ||
- sim_type | ||
- interaction_type | ||
features: [sensor_pos_x, sensor_pos_y, sensor_pos_z, t] | ||
graph_definition: | ||
arguments: | ||
columns: [0, 1, 2] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the naming of this "columns" arguments (which I understand as the columns included in the "distance" calculation of your edge definition) becomes very vague. At first glance it hard to tell what this feature does. Something like "edge_defining_columns" would be more descriptive I think. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that the name of that argument is not unambiguous, but I also fail to find better alternatives that are not very long. The name of this argument is the same as what we used to call it in the KNNGraphBuilder (see here) and I do think that the doc string for this argument (see here) is pretty clear. So even though it's a bit challenging to understand what that argument does when one reads the config file, I think we should keep it as-is and refer users to the docs instead (which is the intended usage anyway). Is that OK with you? @Aske-Rosted There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Completely fine. |
||
detector: | ||
arguments: {} | ||
class_name: Prometheus | ||
dtype: null | ||
nb_nearest_neighbours: 8 | ||
node_definition: | ||
arguments: {} | ||
class_name: NodesAsPulses | ||
node_feature_names: [sensor_pos_x, sensor_pos_y, sensor_pos_z, t] | ||
class_name: KNNGraph | ||
index_column: event_no | ||
truth_table: truth | ||
seed: 21 | ||
selection: null | ||
loss_weight_column: null | ||
loss_weight_default_value: null | ||
loss_weight_table: null | ||
node_truth: null | ||
node_truth_table: null | ||
path: $GRAPHNET/data/examples/sqlite/prometheus/prometheus-events.db | ||
pulsemaps: total | ||
seed: null | ||
selection: null | ||
string_selection: null | ||
truth: [injection_energy, injection_type, injection_interaction_type, injection_zenith, | ||
injection_azimuth, injection_bjorkenx, injection_bjorkeny, injection_position_x, | ||
injection_position_y, injection_position_z, injection_column_depth, primary_lepton_1_type, | ||
primary_hadron_1_type, primary_lepton_1_position_x, primary_lepton_1_position_y, | ||
primary_lepton_1_position_z, primary_hadron_1_position_x, primary_hadron_1_position_y, | ||
primary_hadron_1_position_z, primary_lepton_1_direction_theta, primary_lepton_1_direction_phi, | ||
primary_hadron_1_direction_theta, primary_hadron_1_direction_phi, primary_lepton_1_energy, | ||
primary_hadron_1_energy, total_energy] | ||
truth_table: mc_truth |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that the path is an absolute path instead of a relative path from the graphnet repository. This might be more elegantly dealt with in a separate PR, since it was like this before as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I've looked through the code base and I actually cannot find anything that depends on this config file, so I wonder why we still have it. Perhaps it's worth deleting.