-
Notifications
You must be signed in to change notification settings - Fork 1
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
Fix/visualize results #96
Conversation
There was some logic mismatch with the internal booleans and the strings passed to the function. Basically, all the full strings were evaluated to true, even if the string was "false"
TODO: Fix the font sizes so it looks good
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.
There is still the issue with nmr_only being set to False most of the time, which should not be the case. It seems that str2bool does not resolve this (it should work by passing it as a bool, str2bool should not be required, it is a code logic problem). Also, kindly asking to delete the old results.
metrics/old/results_baselines.csv
Outdated
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.
Please delete this file (old metrics, not needed)
metrics/old/results_multi_target.csv
Outdated
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.
Please delete this file (old metrics, not needed)
metrics/old/results_one_target.csv
Outdated
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.
Please delete this file (old metrics, not needed)
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.
Here, it still only gives “nor_only” = False. It should be True as most of the time, we do NOT want models to be trained with the ligands.
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.
Same problem here — it should train and evaluate the models for “nmr_only” = True, there is still a bug
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.
Kindly delete these comments if the code is not needed anymore
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.
I still do not understand this function and why it should fix the bug
…r/NMRcraft into fix/visualize_results
No description provided.