-
Notifications
You must be signed in to change notification settings - Fork 5
Code Review and Bug Fixes #48
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
Conversation
…ing problem is that the Nexus Data is not yet hosted on the S3 Bucket rest all is working properly after the updates. Another problem that I encountered is that the download button doesn't work as Github doesn't allow raw files to be downloaded. I fixed this by replacing the Github link with the zenodo download link. I have also added the notebook for using Neural Posterior Estimation (NPE) with the nbi (Neural Bayesian Inference) module as a guide for using GPUs and as an alternative to the standard fitting approaches. Updates: Bin section - data_challenge_aas_workshop.md - The session outlines don't have the correct links, which should be updated before the workshop. Also fixed the Jekyll Specific Link Syntaxes to standard HTML anchor tags. The missing publication plans link needs to be updated. - data_challenge_binary_lenses.md - The session intro doesn't have the correct link, and the download button does not work. - data_challenge_binary.md - No content. Why does this exist? - data_challenge_data.md - Same as DATA.md in Session D. Fixed Math input error and the Jekyll style imports to standard HTML anchor tags, and fixed the notebook redirect. - data_challenge_info.md - Will be filled with the information once completed. data_challenge_lauched.md - Redirect links have been corrected to lead to the right pages on the RGES PIT Website. Fixed the Jekyll style imports to standard HTML anchor tags. - data_challenge_microlensing_tools.md - No content. Why does this exist? The download link doesn't work. - data_challenge_nexus_help.md - Same as Session_A Readme.md. Fixed the Jekyll style imports to standard HTML anchor tags, fixed the redirect links in the nexus specific notebooks section. The download button does not work. - data_challenge_pipeline.md - No content. Why does this exist? -data_challenge_single_lenses_and_pipelines.md - Same as Session B Readme.md . Fix the Jekyll style imports to standard HTML anchor tags, download button does not work. - data_challenge_submission_tutorial.md - Fixed the Jekyll style imports to standard HTML anchor tags. - data_challenge_workflow.md - The GitHub Redirect does not lead to a valid page, and the download page doesn't work. AAS Workshop - README.md - Fixed the incorrect GitHub repo link, fixed some indentations, corrected session outline formatting, and updated the path for running the notebooks through Nexus. Session A: Nexus - ReadMe.md - Changed the incorrect repository name. Fixed the Jekyll style imports to standard HTML anchor tags. - Nexus_Workflow.ipynb - The important links were not correctly configured and were updated; the challenges' S3 bucket is still not configured, which means the rest of the notebook cannot be checked. Where is the data_discovery_and_access.md located? Added an appendix on working with Parquet Files. Session B: Single Lens & Pipelines - ReadMe.md - Changed the incorrect repository name. Fixed the Jekyll style imports to standard HTML anchor tags, the download button does not work. - Single_Lens_Pipeline.ipynb - Updated the Full Season Roman Fit section as the columns were not being loaded properly, the challenges' S3 bucket does not load the data Session C: Binary Lenses - Readme.md - Changed the incorrect repository name. The download button does not work. - microlens_emcee.py - No changes required. - Fitting_Binary_Lenses.ipynb - Added a progress bar to the grid search section and added a more detailed description to the grid search functions. Session D: Data_Challenge Q & A - ABOUT.md - Changed the incorrect repository name, changed the Jekyll style imports to standard HTML anchor tags. - DATA.md - Changed the Jekyll style imports to standard HTML anchor tags. - Readme.md - Changed the incorrect repository name, changed the Jekyll style imports to standard HTML anchor tags. Extras - README.md - Fixed the incorrect download button and added an additional section on the NPE notebook which uses nbi for simulated PSPL events and provides inferences for the same. - nbi_roman_simulations.ipynb - A new notebook for doing parameter estimation using Amortized and Sequential Neural Posterior Estimation using a PSPL model with noise simulator through the Neural Bayesian Inference (nbi) package. - Microlensing_Tools.ipynb - Updated multiple sections to work without errors, main addition include updating of the astrometry plotting for the RTModel results, and fixing the popclass code cell to work with the correct version of the asdf library. - Gould_Loeb_Planetary_Event.ipynb - Fixed two cells where the interactive image needed to be created and added the answers to the exercises after the citations section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses critical bug fixes and documentation improvements across the data challenge notebooks repository. The primary focus is replacing Jekyll-specific link syntax with standard HTML anchor tags, fixing broken GitHub download buttons by replacing them with Zenodo links, correcting repository name typos, and adding a new Neural Posterior Estimation notebook.
Key changes:
- Replaced Jekyll
{:target="_blank"}syntax with standard HTML<a>tags throughout documentation - Fixed non-functional GitHub raw file download buttons by replacing with Zenodo archive links
- Corrected typo "reges-pit" → "rges-pit" in repository references
- Added
nbi_roman_simulations.ipynbdemonstrating Neural Bayesian Inference for Roman microlensing
Reviewed changes
Copilot reviewed 44 out of 58 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
bin/*.md |
Standardized link syntax from Jekyll to HTML, fixed download buttons to use Zenodo |
AAS Workshop/*/ReadMe.md |
Corrected repository name typos, updated link syntax |
AAS Workshop/Session C/Fitting_Binary_Lenses.ipynb |
Enhanced grid search with progress bars and improved documentation |
Extras/README.md |
Added NPE notebook section, updated download instructions |
Extras/Gould_Loeb_Planetary_Event.ipynb |
Fixed typo, added exercise answers, corrected cell execution counts |
README.md |
Added Zenodo download section, fixed citation comment markers |
RRN/footer.md |
Removed extraneous code fence markers |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
required for correct processing to create RRN copies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for testing the workshop content. Fresh eye are always a very helpful insight. My chronic misspelling of rges-pit is a real joke.
In future, please take a minimal changes approach to PRs. There are a lot of files being added here and I'm unsure which ones are necessary. Markdown to html changes bulk up the diff unnecessarily. As does the adding and removal of newlines.
I think the confusion around the buttons seems to be because you are clicking them in GitHub. That's not what they are for. They are fed into the rges-pit website as actual functioning buttons on the workshop pages. but a functioning button in GitHub would be pretty much pointless; they are already in the repo to read the readme. Downloading the notebook in the same folder is trivial. And, like I mentioned in our meeting and in my other comments, the Zenodo is not updated automatically, so linking there risks giving people out of date content
| "\n", | ||
| "def grid_search_binary(df, logs_vals, logq_vals, alpha_vals,\n", | ||
| " t0_init, u0_init, tE_init, logrho_init,\n", | ||
| " bounds=None, verbose=False, n_jobs=1):\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason for this change? @Meet-Vyas-Dev @ArjunMurlidhar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a derived file, not a cannonical file and should not be edited, per the README.md.
Derived content is generated from the canonical directories via transformers in
scripts/using thenotebooks_manifest.ymlmanifest. Contributions should not be made directly toRRN/build/orbin/because those directories are regenerated artifacts. SeeRRN/README.mdand the documentation withinbin/for details.
But that's my bad for not also updating the CONTRIBUTING.md guide.
I have made the following changes highlighted below. The most concerning problem is that the Nexus Data is not yet hosted on the S3 Bucket; the rest is working properly after the updates. Another problem that I encountered is that the download button doesn't work, as GitHub doesn't allow raw files to be downloaded. I fixed this by replacing the GitHub link with the Zenodo download link. I have also added the notebook for using Neural Posterior Estimation (NPE) with the nbi (Neural Bayesian Inference) module as a guide for using GPUs and as an alternative to the standard fitting approaches.
The current updates include:
Bin
data_challenge_aas_workshop.md - The session outlines don't have the correct links, which should be updated before the workshop. Also fixed the Jekyll Specific Link Syntaxes to standard HTML anchor tags. The missing publication plans link needs to be updated.
data_challenge_binary_lenses.md - The session intro doesn't have the correct link, and the download button does not work.
data_challenge_binary.md - No content. Why does this exist?
data_challenge_data.md - Same as DATA.md in Session D. Fixed Math input error and the Jekyll style imports to standard HTML anchor tags, and fixed the notebook redirect.
data_challenge_info.md - Will be filled with the information once completed. data_challenge_lauched.md - Redirect links have been corrected to lead to the right pages on the RGES PIT Website. Fixed the Jekyll style imports to standard HTML anchor tags.
data_challenge_microlensing_tools.md - No content. Why does this exist? The download link doesn't work.
data_challenge_nexus_help.md - Same as Session_A Readme.md. Fixed the Jekyll style imports to standard HTML anchor tags, fixed the redirect links in the Nexus-specific notebooks section. The download button does not work.
data_challenge_pipeline.md - No content. Why does this exist?
data_challenge_single_lenses_and_pipelines.md - Same as Session B Readme.md . Fix the Jekyll style imports to standard HTML anchor tags, download button does not work.
data_challenge_submission_tutorial.md - Fixed the Jekyll style imports to standard HTML anchor tags.
data_challenge_workflow.md - The GitHub Redirect does not lead to a valid page, and the download page doesn't work.
AAS Workshop
README.md - Fixed the incorrect GitHub repo link, fixed some indentations, corrected session outline formatting, and updated the path for running the notebooks through Nexus.
Session A: Nexus
ReadMe.md - Changed the incorrect repository name. Fixed the Jekyll style imports to standard HTML anchor tags.
Nexus_Workflow.ipynb - The important links were not correctly configured and were updated; the challenges' S3 bucket is still not configured, which means the rest of the notebook cannot be checked. Where is the data_discovery_and_access.md located? Added an appendix on working with Parquet Files.
Session B: Single Lens & Pipelines
ReadMe.md - Changed the incorrect repository name. Fixed the Jekyll style imports to standard HTML anchor tags, the download button does not work.
Single_Lens_Pipeline.ipynb - Updated the Full Season Roman Fit section as the columns were not being loaded properly, the challenges' S3 bucket does not load the data
Session C: Binary Lenses
Readme.md - Changed the incorrect repository name. The download button does not work.
microlens_emcee.py - No changes required.
Fitting_Binary_Lenses.ipynb - Added a progress bar to the grid search section and added a more detailed description to the grid search functions.
Session D: Data_Challenge Q & A
ABOUT.md - Changed the incorrect repository name, changed the Jekyll style imports to standard HTML anchor tags.
DATA.md - Changed the Jekyll style imports to standard HTML anchor tags.
Readme.md - Changed the incorrect repository name, changed the Jekyll style imports to standard HTML anchor tags.
Extras
README.md - Fixed the incorrect download button and added an additional section on the NPE notebook which uses nbi for simulated PSPL events and provides inferences for the same.
nbi_roman_simulations.ipynb - A new notebook for doing parameter estimation using Amortized and Sequential Neural Posterior Estimation using a PSPL model with noise simulator through the Neural Bayesian Inference (nbi) package.
Microlensing_Tools.ipynb - Updated multiple sections to work without errors, main addition include updating of the astrometry plotting for the RTModel results, and fixing the popclass code cell to work with the correct version of the asdf library.
Gould_Loeb_Planetary_Event.ipynb - Fixed two cells where the interactive image needed to be created and added the answers to the exercises after the citations section.