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

Add ROCES and NCES2 #511

Merged
merged 28 commits into from
Feb 4, 2025
Merged

Add ROCES and NCES2 #511

merged 28 commits into from
Feb 4, 2025

Conversation

Jean-KOUAGOU
Copy link
Collaborator

Both ROCES and NCES2 are integrated in Ontolearn now.

What is left?:

Train NCES2 and ROCES and upload pretrained models to dice-files-system.

@Jean-KOUAGOU Jean-KOUAGOU requested a review from Demirrr January 21, 2025 15:47
@github-actions github-actions bot temporarily deployed to pull request January 21, 2025 15:57 Inactive
@github-actions github-actions bot temporarily deployed to pull request January 22, 2025 15:29 Inactive
@github-actions github-actions bot temporarily deployed to pull request January 22, 2025 15:49 Inactive
@github-actions github-actions bot temporarily deployed to pull request January 23, 2025 17:05 Inactive
Copy link
Collaborator

@alkidbaci alkidbaci left a comment

Choose a reason for hiding this comment

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

That's quiet a large amount of changes. Overall things look good. I have a few comments on this PR:

  • Add the boolean argument return_node to the method best_hypotheses where user can select to return the node or the CE (the return type should also be updated). I noticed that you save the nodes in the class argument best_predictions but in order to keep the same signature for this method throughout our learning models I recommend updating it.
  • You have 2 implementations for static method generate_training_data which are very similar. You can optimize this current state by merging them into a single method and it can be moved to nces_utils.py.
  • Please add docstrings for every method\class that does not inherit any docstring from a base class. This information shows in the API at our docs and it helps everyone into better understanding your code and how they can use it.

@Demirrr
Copy link
Member

Demirrr commented Jan 31, 2025

@alkidbaci please review it

@github-actions github-actions bot temporarily deployed to pull request February 3, 2025 13:36 Inactive
@github-actions github-actions bot temporarily deployed to pull request February 3, 2025 13:44 Inactive
@github-actions github-actions bot temporarily deployed to pull request February 3, 2025 13:56 Inactive
@github-actions github-actions bot temporarily deployed to pull request February 3, 2025 20:06 Inactive
@alkidbaci alkidbaci merged commit 1f36cd0 into develop Feb 4, 2025
2 of 4 checks passed
@github-actions github-actions bot temporarily deployed to pull request February 4, 2025 12:36 Inactive
@alkidbaci alkidbaci deleted the add_roces_nces2 branch February 5, 2025 10:56
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.

3 participants