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

Model Vis inline with Callback ipynb #13

Open
wants to merge 90 commits into
base: examples
Choose a base branch
from

Conversation

Xyzic
Copy link
Contributor

@Xyzic Xyzic commented Jun 1, 2022

Updated modelvis to be streamlined and a bunch of other files for testing/examples.

Updated gitignore to disregard wip stuff and already existing examples

matthewturk and others added 30 commits October 18, 2021 10:07
So far it's only workable with the photosynthesis model. Once it shows up in the node editor I can work more on optimizing it, getting rid of hard coded stuff, etc
Still a little hard-coded, but I can work on that once it's working and cleaned up properly
Big code cleanup
@Xyzic Xyzic changed the title Added modelvis + models/dependencies Model Vis inline with Callback ipynb Jun 1, 2022
@Xyzic
Copy link
Contributor Author

Xyzic commented Jul 19, 2022

Added in new notebook, modified a few core files as requested

@matthewturk
Copy link
Member

I have a couple changes, but one that is pretty important -- socket_type should not be connection. I don't completely know why, but that seems to be a reserved word that breaks things and prevents clicking and creating new connections. Change it to bytes. But longer term, we actually need to parse this from the yggdrasil model.

@matthewturk
Copy link
Member

A couple additional things:

  • Format the code with black
  • I don't think we should be returning any instances. The model_loader function should merely transform the input yaml into a set of dict objects that can be supplied to add_component. This helps to make sure that the SocketCollection instance that's used by all the different components is the same.
  • We need to parse the data types being accepted and emitted by the different slots. That means using the default that yggdrasil assumes, if we need to; this is one (of several) instances that relying on yggdrasil's normalization code would be helpful. (@langmm may have thoughts on how to have it fill out the defaults without instantiating any compilation or execution procedures.)
  • I believe you can considerably simplify the dictionary translation code.

@Xyzic
Copy link
Contributor Author

Xyzic commented Aug 9, 2022

Everything done except the third bullet and probably more can be done for the fourth.

@Xyzic
Copy link
Contributor Author

Xyzic commented Aug 15, 2022

Quick comment on the socket types -- this should always be bytes as Yggdrasil always returns bytes, so there shouldn't need to be a need to parse out the actual type. The code does that in a list just in case, but in all test cases so far it has been bytes

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.

2 participants