Skip to content
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

Fixed error related to new model saving/loading method and early stopping method #81

Merged
merged 7 commits into from
Nov 22, 2021

Conversation

RasmusOrsoe
Copy link
Collaborator

No description provided.

@@ -208,7 +208,7 @@ def _forward(self, prediction: Tensor, target: Tensor) -> Tensor:

# Formatting prediction
angle_pred = prediction[:,0]
kappa = prediction[:,1]
kappa = abs(prediction[:,1])
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this abs statement is necessary, as it would imply that the model (in this case, Task components) might return a negative value for kappa, which would not be meaningful. If that is the case, it should be fixed in the corresponding model/task. Therefore, I suggest we leave it to the model to impose the correct restrictions on the output, and in the loss function(s) assume that inputs are as expected.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have removed the abs() from the PR as its besides the point for what this PR does. I do think theres merit to adding the abs(), but lets talk about that some other time

@RasmusOrsoe RasmusOrsoe merged commit ee5a626 into graphnet-team:main Nov 22, 2021
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