Skip to content

Conversation

@ismael-mendoza
Copy link
Contributor

@ismael-mendoza ismael-mendoza commented Jun 26, 2020

This PR will revive the Scarlet tutorial so that it runs with the latest version of scarlet installed in the Stack. Resolves #166

The last part of the tutorial Update Functions seems deprecated in a way that I can't quite figure out how to fix. It seems that this has to do with using different constrains, although it's possibly done in a very different way now. Maybe remove this part?

@ismael-mendoza ismael-mendoza added the Deblending Topic area, to help people find projects to work on label Jun 26, 2020
@ismael-mendoza ismael-mendoza self-assigned this Jun 26, 2020
@ismael-mendoza ismael-mendoza marked this pull request as draft June 26, 2020 16:54
@kadrlica kadrlica requested a review from fred3m June 26, 2020 17:31
@kadrlica
Copy link
Contributor

@fred3m would you be willing to take a look at this?

Copy link

@fred3m fred3m left a comment

Choose a reason for hiding this comment

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

Sure.

@ismael2395 could you please remove the images, rebase, and force push the changes? It's hard to see what's changed with all of the images in the notebook, plus it will take up too much space if we always save images in the repo.

But I looked at the notebook version and on first glance it looks good but I have a few additional change requests:

  • I would change e_rel to 1e-4 or 1e-5 to allow scarlet to run for a few more iterations. There's a lot of flux leftover in the residuals that it should be able to model with another 20-30 iterations. Or possibly leave it as it and invite the user to modify e_rel to higher and lower values to better understand how it affects the results.

  • The update cell is no longer useful because we no longer do constraints that way. So for now I'd recommend just deleting those cells and if you want to open up a ticket and assign it to me I'll create a new cell or two that illustrates how to apply constraints in the newest version of scarlet.

Thanks for keeping the notebook up to date!

@ismael-mendoza
Copy link
Contributor Author

Thanks @fred3m for your suggestions. I incorporated them and I now I'm just trying to rebase and merge - I might close this PR instead and start a new one to avoid having to force push

@ismael-mendoza ismael-mendoza deleted the u/imendoza/scarlet branch July 10, 2020 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Deblending Topic area, to help people find projects to work on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Project: Getting the deblending tutorials to successfully run

4 participants