QueuQ / CGLB

Other
50 stars 15 forks source link

GEM baseline in class-IL for GCGL has wrong indentation for optimizer step? #11

Closed WeiWeic6222848 closed 1 year ago

WeiWeic6222848 commented 1 year ago

as seen in the next code fragment, line 212, optimizer.step() is outside of the for loop, which iterates over the current task. https://github.com/QueuQ/CGLB/blob/3e0debf02e582610d05274b44c4c09fc1c1fe4b2/GCGL/Baselines/gem_model.py#L178-L212 while in the same file, in line 296 and line 127, this step is inside the for loop.

QueuQ commented 1 year ago

as seen in the next code fragment, line 212, optimizer.step() is outside of the for loop, which iterates over the current task.

https://github.com/QueuQ/CGLB/blob/3e0debf02e582610d05274b44c4c09fc1c1fe4b2/GCGL/Baselines/gem_model.py#L178-L212

while in the same file, in line 296 and line 127, this step is inside the for loop.

Thanks for pointing out this issue. It should be a mistake, and we have corrected it and updated it. We tested the result of GEM with the updated code (on Aromaticity-CL, the only class-IL dataset), and got AP 5.2±1.1 AF -57.6±3.4. The AP decreases a little, and the AF increases. The previous code would skip some batches of data in each epoch, but since shuffle is used for the data loader, all data should get chance to be trained after multiple epochs. Now the corrected code would ensure that all batches are used in each epoch.

WeiWeic6222848 commented 1 year ago

Thank you for the update