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

Restore old MLL fit #320

Merged
merged 2 commits into from
Jul 24, 2024
Merged

Restore old MLL fit #320

merged 2 commits into from
Jul 24, 2024

Conversation

Scienfitz
Copy link
Collaborator

use torch fitting with 100 steps again until we finetune this better

here some pictures with the new fit indicating previous results are there again
image
image
image
image

tobis notebook with this PR
image

vs old (non torch) fitting
image

@Scienfitz Scienfitz added the bug Something isn't working label Jul 22, 2024
@Scienfitz Scienfitz self-assigned this Jul 22, 2024
@AdrianSosic
Copy link
Collaborator

Hi @Scienfitz, thanks for the change, looking good 👍🏼 I suggest that we wait with the merge until the scaling is in, to have this as a last sanity check before the new release (please keep the files so that we can rerun again once everything is in place!)

@Scienfitz
Copy link
Collaborator Author

@AdrianSosic please can I merge this? I need to move forward with my items, and the block here seems artificial as the PR has nothing to do with running the tests which you or I are perfectly capable of doing independently of this PR

@Scienfitz Scienfitz force-pushed the fix/mll_fit branch 2 times, most recently from 15f17e4 to a68c2a7 Compare July 23, 2024 18:10
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@AdrianSosic AdrianSosic left a comment

Choose a reason for hiding this comment

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

Ok, if it helps you to progress faster, let me accept. But I think the tests are related to this PR because who nows if stuff still works as shown here once all changes are in? So let's definitely have last final test before the release, which potentially requires adjusting the approach once again

@Scienfitz Scienfitz merged commit d7c87a4 into main Jul 24, 2024
9 of 10 checks passed
@Scienfitz Scienfitz deleted the fix/mll_fit branch July 24, 2024 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants