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

.NET Polyglot Notebook and Jupyter Notebook samples #89

Closed
dcuccia opened this issue Aug 26, 2023 Discussed in #70 · 17 comments
Closed

.NET Polyglot Notebook and Jupyter Notebook samples #89

dcuccia opened this issue Aug 26, 2023 Discussed in #70 · 17 comments
Assignees

Comments

@dcuccia
Copy link
Contributor

dcuccia commented Aug 26, 2023

This is a continuation ticket from #72, "Implement and test script-style use of the VTS", and originates from this Discussion: #70

Now that we have script-style C# files in Vts.Scripting which are compiled and tested alongside the rest of the VTS tools, we would now like to create initial .Net Polyglot Notebook and Jupyter Notebook implementations of these scripts. The goal is that the body of the Vts.Scripting scripts should be copy-pastable, but the helpers, library references, and "global" using statements can be handled in a header code block.

Notebooks will also allow us more flexibility to provide markdown-style formatted text in-between code blocks, so that we can be more expressive with a demonstration/walk-through, and to break up a computation into logical parts where useful. And example of this would be running creation of a photon database in one block, and then running desired post-processing in a separate block. Segregation helps in this case because the post-processing can be tweaked and augmented without having to re-run the initial simulation, and the variables and files generated with a photon database can be re-generated with a larger photon count, once the range of post-processing tasks is ironed out.

While the ultimate goal is to have the notebooks live in their own small, separate repositories, in order to flush-out the desired folder/file structure, and identify additional core Vts additions (or bug fixes) needed for making notebook use simple, we will start by adding a small set of prototype notebooks to the VTS project repository itself. These will reference a local Vts.dll library (required to be pre-built from within the Vts.dll project), as opposed to reference them from Nuget. This way, we can rapidly add any new functionality needed to make the look and feel of Vts use as "idiomatic" as possible from within the notebook tools and when used from non-C# languages such as Python (and it's constellation of tools like NumPy, SciPy, etc). For example, this may motivate new array indexing styles and multi-dimensional array helpers like Span2D, adoption of libraries such as NumPy.NET, NumSharp and NumSharp.Bitmap.

Finally, per here we'll re-organize the root 'Matlab' folder into a sub-folder called examples, so that we can have a range of demos of framework use, including scripting/Matlab, scripting/Python, scripting/CSharp (Polyglot Notebooks), maybe even scripting/JavaScript. :)

@dcuccia dcuccia self-assigned this Aug 26, 2023
@lmalenfant
Copy link
Member

@dcuccia I want to make sure this doesn't drop off the radar due to the exciting reconfiguration of detectors we are working on. This has priority over the reconfiguration and we can take the time we need for the reconfiguration discussion.

@dcuccia
Copy link
Contributor Author

dcuccia commented Oct 10, 2023

Yeah, sorry. It's a different beast, as I was having concerns that my computer setup was the thing causing bugs. Was going to create an isolated instance in Docker to see if I could debug that, but haven't had a chance to dig in. Perhaps you can see if the Python notebook runs as-is on your machine?

@lmalenfant
Copy link
Member

lmalenfant commented Oct 10, 2023

Perhaps you can see if the Python notebook runs as-is on your machine?

I'll take a look.

@lmalenfant
Copy link
Member

lmalenfant commented Oct 10, 2023

It does not run as is but with a little tweaking I was able to get it to work:
image

There was an error:
\AppData\Local\Temp\ipykernel_26556\3487594450.py:22: RuntimeWarning: divide by zero encountered in log
logReflectance = [np.log(r) for r in detectorResults[0].Mean]

@dcuccia
Copy link
Contributor Author

dcuccia commented Oct 10, 2023

Oh fantastic! What did you change,?

@lmalenfant
Copy link
Member

Just the path, it doesn't like relative path for some reason.

@dcuccia
Copy link
Contributor Author

dcuccia commented Oct 10, 2023

Great. We'll need a routine to dynamically grab the root directory, but maybe that's all we needed. If you wouldn't mind sharing your change that got it working, I can finish up the demo and PR it.

@lmalenfant
Copy link
Member

lmalenfant commented Oct 10, 2023

dotnet publish doesn't appear to work in the Jupyter notebook so I pointed it to the publish folder that is created after running BuildTestCore.ps1. I added a comment to run the PowerShell script.

I committed the notebook but the way the notebooks show in SonarCloud is as binary files, it looks fine in GitHub though.

@lmalenfant
Copy link
Member

lmalenfant commented Oct 10, 2023

I am concerned how large that one Jupyter notebook is, it is 3.7MB. Is there any way to check and see how they are handled in GitHub because if they as considered binary that would be a 3.7MB commit for every file change. We have had issues previously with large files in the repository and I want to make sure we are aware of content that could considerably increase the size of the repo.

@dcuccia
Copy link
Contributor Author

dcuccia commented Oct 13, 2023

Sorry for the delayed reply. Yeah - forgot to warn you about that. After you run a Jupyter file, you have to "clear" the file before committing to source code. It's a computed data storage medium as well as source code, which makes them portable documents that don't necessarily need to be re-run by the viewer, but that doesn't play nice with repositories. Microsoft tried to encourage a source-friendly alternative and added the "source-only" .dib file format in their notebooks (they also support .ipynb), but .ipynb is still the standard in the community.
image

@lmalenfant
Copy link
Member

lmalenfant commented Oct 17, 2023

@dcuccia, after reviewing this branch last week, in addition to the large notebook file (thank you for the sharing the fix for that), I discovered that there were 293 file changes. Could you check your line ending settings for git because it looks like it modified the line endings on all the MATLAB files.

I am also creating a discussion for this branch because I don't think the folder renaming will work given that we plan to move the notebooks out of the main repository. We cannot move the MATLAB scripts out of the repo because we need them to create the MATLAB release zip and validate the MCCL when running BuildTestRelease. I will create that discussion today.

We already have an ongoing discussion:
#70

@dcuccia
Copy link
Contributor Author

dcuccia commented Oct 17, 2023

Maybe we should do a little work to decouple Matlab from testing MCCL? I'd much rather have a uniform approach to the same type of problem (store in same repo, or in separate repos), than have a patchwork solution. And also generally, decoupling scripts vs core libraries vs apps (MCCL) would help significantly with this project's agility and ability to release without as much ceremony/work.

@lmalenfant
Copy link
Member

@dcuccia I would really like to get the VTS code changes for this branch into master before we do the release so we can work on splitting the code into a separate repository. Let me know if you would like me to assist on this branch so we can get it across the finish line.

If you would like me to clean up the branch as suggested here, I would be happy to do that:
#70 (reply in thread)

@dcuccia
Copy link
Contributor Author

dcuccia commented Oct 23, 2023

Thanks @lmalenfant. I haven't had time to investigate the line endings in detail. If you wouldn't mind adjusting those, I'd appreciate it. The Matlab files were edited slightly to match the comments/naming/values across scripts, so I don't want to lose that. If we must move them back to a root /matlab folder, let's at least be sure not to lose the improvements we've made to the build/test scripts that reference the root matlab folder in a portable way. As an alternative to making folder modifications, we could consider the new discussion regarding segregation of matlab as a "continuation ticket" to further refine what's here. Up to you.

@lmalenfant
Copy link
Member

@dcuccia No problem, I'm on it, I will make sure I keep the improvements in place. I do think the MATLAB segregation needs a bigger discussion (#117) and is not something I think we can resolve before the release. I put in my initial thoughts when I started the discussion, could you share yours there also?

@lmalenfant lmalenfant self-assigned this Oct 25, 2023
@lmalenfant
Copy link
Member

@dcuccia I created 2 repositories https://github.com/VirtualPhotonics/Vts.Scripting.CSharp and https://github.com/VirtualPhotonics/Vts.Scripting.Python for the Jupyter notebooks.

The C# notebook now works because it is getting the VTS library from NuGet, there are several warnings but it plots.

The Python notebook will not work as is, we need to copy the VTS library files into a "libraries" folder and then it will work (this also has a warning that I mentioned previously). I didn't want to commit the dlls to the repo, I would like to find a solution that can get the library similarly to the C# sample, if you have any ideas let me know. I have searched but have still not found an elegant solution.

Since we already added the code changes from the branch associated with this issue and now we have these 2 repositories with the notebooks, I would like to delete this branch and close this issue. I will replace it with a duplicate issue on each of the new repositories to continue working on the scripts.

@lmalenfant
Copy link
Member

Added issues to the new repositories so we can close this issue:
VirtualPhotonics/Vts.Scripting.CSharp#1
VirtualPhotonics/Vts.Scripting.Python#1

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

No branches or pull requests

2 participants