Skip to content

Conversation

@edoardolegnaro
Copy link
Contributor

@edoardolegnaro edoardolegnaro commented Jan 29, 2025

Hierarchical Multi-Task Learning for McIntosh classification of magnetogram cutouts
(currently prints values, will be updated with logging).

loss_z = criterion_z(output_z, labels_z)
loss_p = criterion_p(output_p, labels_p)
loss_c = criterion_c(output_c, labels_c)
loss = loss_z + loss_p + loss_c
Copy link
Contributor

Choose a reason for hiding this comment

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

So not all combinations are valid classification how can we heavily penalise invalid combination during training and exclude them at inference? E.g the table about 1/3 way down this page https://www.stce.be/educational/classification

Comment on lines +63 to +67
# Hierarchical classification heads
# Z -> P -> C
self.fc_Z = nn.Linear(224, num_classes_Z)
self.fc_P = nn.Linear(224 + num_classes_Z, num_classes_P)
self.fc_C = nn.Linear(224 + num_classes_Z + num_classes_P, num_classes_C)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice is there publication associated with this approach?

Comment on lines 185 to 191
for z, p, c in valid_combined_classes:
z_idx = encoders["Z_encoder"].transform([z])[0]
p_idx = encoders["p_encoder"].transform([p])[0]
c_idx = encoders["c_encoder"].transform([c])[0]

valid_p_for_z[z_idx].add(p_idx) # Map Z to valid P
valid_c_for_zp[(z_idx, p_idx)].add(c_idx) # Map (Z, P) to valid C
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if the data set will contain all valid classes:

  1. It could have invalid classes need to check the code we use to filter
  2. It could be missing a valid class due to split or rarity e.t.c

Comment on lines +128 to +139
"D": "LG", # Merge D, E, F into LG (LargeGroup)
"E": "LG",
"F": "LG",
"H": "H",
}

p_component_mapping = {
"x": "x",
"r": "r",
"s": "sym", # Merge s and h into sym
"h": "sym",
"a": "asym", # Merge a and k into asym
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there not enough example for these classes?

Copy link
Contributor

@samaloney samaloney left a comment

Choose a reason for hiding this comment

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

Looks good apart from setup.cfg conflicts

Copy link
Contributor

@samaloney samaloney Feb 10, 2025

Choose a reason for hiding this comment

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

Think the conflicts with this file need to be resolved - also the dev section should be for tool required to develop the code e.g. black, flake8 if it required to run should be in model or install sections or similar but we can tackle this in a different PR

@samaloney
Copy link
Contributor

@edoardolegnaro I'd like to get this merged can you resolve the conflicts?

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.

2 participants