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

Address issue #113 #119

Merged
merged 1 commit into from
Mar 4, 2022
Merged

Address issue #113 #119

merged 1 commit into from
Mar 4, 2022

Conversation

jinlong83
Copy link
Contributor

@jinlong83 jinlong83 commented Mar 3, 2022

The purposes of this pull request are:

  • Update the implementation of sparse EKI to make it compatible with the new version of DataContainer.

@odunbar
Copy link
Collaborator

odunbar commented Mar 3, 2022

Looks good! could you merge base in? Then I'm happy to have this merged in

@ilopezgp
Copy link
Contributor

ilopezgp commented Mar 3, 2022

Looks good! could you merge base in? Then I'm happy to have this merged in

I think this addresses #113 but not #117. Let's wait until both are resolved or change the name of the PR.

@ilopezgp
Copy link
Contributor

ilopezgp commented Mar 3, 2022

It might be a good idea to limit this PR to #113 so we can merge it quickly. I am not sure how involved the other one will be @jinlong83 .

@jinlong83
Copy link
Contributor Author

It might be a good idea to limit this PR to #113 so we can merge it quickly. I am not sure how involved the other one will be @jinlong83 .

I agree. I saw that Convex v0.14 has about 20 releases, and v0.15.0 just came out one day ago? I'd suggest stay with v0.14.x for now if possible, and I'll play with v0.15.x to better understand the issue of #117

@jinlong83 jinlong83 changed the title [WIP] Address issues #113 and #117 [WIP] Address issue #113 Mar 3, 2022
@ilopezgp
Copy link
Contributor

ilopezgp commented Mar 4, 2022

Thank you @jinlong83 ! Could you squash your commits before we merge? Thanks!

@ilopezgp ilopezgp changed the title [WIP] Address issue #113 Address issue #113 Mar 4, 2022
@jinlong83 jinlong83 force-pushed the jw/sparse_eki branch 2 times, most recently from f141bfd to 32c0105 Compare March 4, 2022 01:58
@ilopezgp
Copy link
Contributor

ilopezgp commented Mar 4, 2022

bors r+

@bors
Copy link
Contributor

bors bot commented Mar 4, 2022

Canceled.

bors bot added a commit that referenced this pull request Mar 4, 2022
119: Address issue #113 r=ilopezgp a=jinlong83

The purposes of this pull request are:

- Update the implementation of sparse EKI to make it compatible with the new version of DataContainer.

Co-authored-by: Jinlong <[email protected]>
Co-authored-by: Jinlong <[email protected]>
@ilopezgp
Copy link
Contributor

ilopezgp commented Mar 4, 2022

bors r+

bors bot added a commit that referenced this pull request Mar 4, 2022
119: Address issue #113 r=ilopezgp a=jinlong83

The purposes of this pull request are:

- Update the implementation of sparse EKI to make it compatible with the new version of DataContainer.

Co-authored-by: Jinlong <[email protected]>
Co-authored-by: Jinlong <[email protected]>
@ilopezgp
Copy link
Contributor

ilopezgp commented Mar 4, 2022

bors r-

@bors
Copy link
Contributor

bors bot commented Mar 4, 2022

Canceled.

@ilopezgp
Copy link
Contributor

ilopezgp commented Mar 4, 2022

I don't think you need to merge from master, just squash. Bors makes sure everything is ok before merging if there are no conflicts

updated the implementation with function sparse_eki_update.

minor change of format.

minor change of comments.

minor change of comments.
@jinlong83
Copy link
Contributor Author

bors r+

@bors
Copy link
Contributor

bors bot commented Mar 4, 2022

Build succeeded:

@bors bors bot merged commit ca1f50e into main Mar 4, 2022
@bors bors bot deleted the jw/sparse_eki branch March 4, 2022 02:28
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