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

initial commit of SMARTS NN notebook #13

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

zclaytor
Copy link

No description provided.

@cmurray-astro
Copy link
Collaborator

Hi @zclaytor! Apologies for the delay in getting this review started. I've gone through the notebook and I have a few questions:

  1. First, I ran into an error with using the "001/" group of simulated light curves (an index error with one of the files, shown in the comments of the notebook pushed here). When I edited to "002/" it all ran well. This is something we should verify with the HLSP ingest side.
  2. It would be great to understand a bit more what your goals are for the notebook? If you can add a few words of comments before each cell, that will help me a lot with drafting the rest of the notebook.
  3. Do you expect the plot at the end of the notebook to look correlated? As with other Hello Universe examples, we don't promise that on-the-fly training with small datasets will get good results, but I just want to check with you that that's what you had in mind?

Thanks again for submitting this, looking forward to hearing from you!

@zclaytor
Copy link
Author

zclaytor commented Jun 7, 2023

Hi @cmurray-astro! Thanks for your questions.

  1. I isolated the issue to a corrupted file, simulation #408. I reran the sim and have a new tar file to upload. Should I go ahead and upload it?
  2. I've added some cell comments following the "Classifying TESS Flares" notebook as an example. Hopefully this clarifies the goals but please let me know if I should add more.
  3. It's totally fine that the plot at the end isn't correlated. Since we're just going over a handful of training examples, I don't expect the tutorial CNN to generalize well.

Thanks!

@cmurray-astro
Copy link
Collaborator

Hi Zach, thanks so much for the update! Yes, please go ahead and upload the updated tar file, and I'll process the HLSP update. In the meantime I'll review the notebook!

@zclaytor
Copy link
Author

Great, thanks! I uploaded the new tar file. I have also verified that the rest of the files are free from this error.

@cmurray-astro
Copy link
Collaborator

hi @zclaytor, I'm working through the notebook, and am running into issues with the training step. Can you confirm which SMARTS datasets you downloaded+used for this case, and also please let me know which versions of the required python packages you are running (really just torch)

thanks!

@cmurray-astro
Copy link
Collaborator

and I should clarify -- the issue is that the notebook kernel dies when it hits the output = model(data) step in the train_epoch loop

@cmurray-astro cmurray-astro marked this pull request as draft December 15, 2023 14:40
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.

None yet

2 participants