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

pcqm4m changes with mpnn and gps++ configs, fix run_validation_test.py #419

Merged
merged 13 commits into from
Aug 1, 2023

Conversation

zhiyil1230
Copy link
Contributor

@zhiyil1230 zhiyil1230 commented Jul 25, 2023

Changelogs

  • enumerate the changes of that PR.

Checklist:

  • Was this PR discussed in an issue? It is recommended to first discuss a new feature into a GitHub issue before opening a PR.
  • Add tests to cover the fixed bug(s) or the new introduced feature(s) (if appropriate).
  • Update the API documentation is a new function is added, or an existing one is deleted.
  • Write concise and explanatory changelogs above.
  • If possible, assign one of the following labels to the PR: feature, fix or test (or ask a maintainer to do it for you).

discussion related to that PR
The mpnn and gpss++ are not up to date with the recent changes. Update them accordingly. Test run with 8000 samples look fine, loss decreasing.

@codecov
Copy link

codecov bot commented Jul 25, 2023

Codecov Report

Merging #419 (f60b968) into main (7898633) will increase coverage by 2.87%.
Report is 10 commits behind head on main.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #419      +/-   ##
==========================================
+ Coverage   63.89%   66.77%   +2.87%     
==========================================
  Files          82       82              
  Lines        7813     7819       +6     
==========================================
+ Hits         4992     5221     +229     
+ Misses       2821     2598     -223     
Flag Coverage Δ
unittests 66.77% <ø> (+2.87%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
ipu 47.70% <ø> (+29.09%) ⬆️

Copy link
Collaborator

@DomInvivo DomInvivo left a comment

Choose a reason for hiding this comment

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

Can you move to the new type of compositional configurations, using hydra?

@zhiyil1230 zhiyil1230 changed the title pcqm4m changes with mpnn and gps++ configs pcqm4m changes with mpnn and gps++ configs, fix run_vali_test.py Jul 27, 2023
@zhiyil1230
Copy link
Contributor Author

Can you move to the new type of compositional configurations, using hydra?

I've modified the pcqm mpnn and gpspp configs to hydra and tested ok. For the largemix configs, I think @joao-alex-cunha is working on it. So here it is just a simple fix.

@zhiyil1230 zhiyil1230 changed the title pcqm4m changes with mpnn and gps++ configs, fix run_vali_test.py pcqm4m changes with mpnn and gps++ configs, fix run_validation_test.py Jul 28, 2023
@DomInvivo DomInvivo merged commit 66c7adc into main Aug 1, 2023
5 checks passed
@notion-workspace
Copy link

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