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.

Sign up for GitHub

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

Jump to bottom

[Refactor] Merge modules losses, distance, and metrics to one #402

Open
wenruifan opened this issue Sep 26, 2023 · 13 comments · May be fixed by #441
Open

[Refactor] Merge modules losses, distance, and metrics to one #402

wenruifan opened this issue Sep 26, 2023 · 13 comments · May be fixed by #441
Assignees
@shuo-zhou @SinaTabakhi @Mdnaimulislam @wenruifan
Labels
refactor Refactor of existing code

Comments

@wenruifan
Copy link
Contributor

wenruifan commented Sep 26, 2023 •
edited by shuo-zhou
Loading

Merge modules losses, distance, and metrics to one.

Modules to be merged

  • kale.utils.distance
  • kale.predict.losses
  • kale.evaluate.metrics

Impact on code structure

The functions in these modules performed similarly, like calculating loss, accuracy, etc. It could be confusing when people try to use them or contribute to the library.
The import parts in some files may be changed.
@wenruifan wenruifan added the refactor Refactor of existing code label Sep 26, 2023
@wenruifan wenruifan changed the title [Refactor] [Refactor] Merge some file for similar functions Sep 26, 2023
@xianyuanliu xianyuanliu changed the title [Refactor] Merge some file for similar functions [Refactor] Merge some files for similar functions Oct 3, 2023
@xianyuanliu
Copy link
Member

xianyuanliu commented Oct 4, 2023 •
edited
Loading

Discussion #399 from @SinaTabakhi mentioned the same issue, which is helpful and constructive. We need to close #399 as well, once this issue is resolved.

From Sina:

Considering the presence of multiple modules in PyKale focused on evaluation metrics, it may be beneficial to merge them.

The following two modules should be merged into one, with only metrics.py remaining:
metrics.py
losses.py

Additionally, when PR #361 is merged into PyKale, the distance.py module should also be merged into metrics.py.

The implementation of pairwise metrics in scikit-learn can help in effectively refactoring these modules.
pairwise.py in scikit-learn

@xianyuanliu
Copy link
Member

Shall we use some metrics in TorchMetrics instead of implementing them ourselves into PyKale?

@shuo-zhou shuo-zhou changed the title [Refactor] Merge some files for similar functions [Refactor] Merge modules losses, distance, and metrics to one Oct 17, 2023
@github-actions GitHub Actions
Copy link
Contributor

This issue has been automatically marked as stale due to lack of activity. It will be closed if no further activity occurs. Thank you!

@github-actions GitHub Actions
Copy link
Contributor

This issue has been automatically marked as stale due to lack of activity. It will be closed if no further activity occurs. Thank you!

@github-actions GitHub Actions
Copy link
Contributor

This issue has been automatically marked as stale due to lack of activity. It will be closed if no further activity occurs. Thank you!

@github-actions GitHub Actions
Copy link
Contributor

This issue has been automatically marked as stale due to lack of activity. It will be closed if no further activity occurs. Thank you!

@github-actions GitHub Actions
Copy link
Contributor

This issue has been automatically marked as stale due to lack of activity. It will be closed if no further activity occurs. Thank you!

@github-actions GitHub Actions
Copy link
Contributor

This issue has been automatically marked as stale due to lack of activity. It will be closed if no further activity occurs. Thank you!

@github-actions GitHub Actions
Copy link
Contributor

This issue has been automatically marked as stale due to lack of activity. It will be closed if no further activity occurs. Thank you!

@github-actions GitHub Actions
Copy link
Contributor

This issue has been automatically marked as stale due to lack of activity. It will be closed if no further activity occurs. Thank you!

@shuo-zhou shuo-zhou linked a pull request Jul 8, 2024 that will close this issue
Open
5 tasks
@shuo-zhou shuo-zhou linked a pull request Jul 8, 2024 that will close this issue
402 refactor merge modules losses distance and metrics to one #441
Open
5 tasks
@github-actions GitHub Actions
Copy link
Contributor

This issue has been automatically marked as stale due to lack of activity. It will be closed if no further activity occurs. Thank you!

@github-actions GitHub Actions
Copy link
Contributor

This issue has been automatically marked as stale due to lack of activity. It will be closed if no further activity occurs. Thank you!

@github-actions GitHub Actions
Copy link
Contributor

This issue has been automatically marked as stale due to lack of activity. It will be closed if no further activity occurs. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@shuo-zhou shuo-zhou

@SinaTabakhi SinaTabakhi

@Mdnaimulislam Mdnaimulislam

@wenruifan wenruifan

Labels
refactor Refactor of existing code
Projects
PyKale v0.2.0
Status: In progress
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

402 refactor merge modules losses distance and metrics to one pykale/pykale
5 participants
@shuo-zhou @SinaTabakhi @xianyuanliu @Mdnaimulislam @wenruifan