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

Create a visual distinction for data flow into Calculators and Distributions #76

Merged
merged 5 commits into from
Aug 18, 2023

Conversation

GianmarcoCallegher
Copy link
Contributor

No description provided.

@GianmarcoCallegher GianmarcoCallegher self-assigned this Aug 2, 2023
@GianmarcoCallegher GianmarcoCallegher added comp:model This issue is related to the model module enhancement New feature or request labels Aug 2, 2023
@jobrachem jobrachem self-requested a review August 9, 2023 14:30
@jobrachem
Copy link
Contributor

I would like to see what the result looks like ;)

@GianmarcoCallegher
Copy link
Contributor Author

GianmarcoCallegher commented Aug 12, 2023

import liesel.model as lsl
import tensorflow_probability.substrates.jax.distributions as tfd
import jax.numpy as jnp

n0 = lsl.Param(1.0, None, 'n0')
n1 = lsl.Var(2.0, None, 'n1')
n2 = lsl.Var(lsl.Calc(jnp.exp, n1), None, 'n2')

n0.dist_node = lsl.Dist(tfd.HalfNormal, scale=n2)

gb = lsl.GraphBuilder()
gb.add(n0, n1, n2)
model = gb.build_model()

lsl.plot_vars(model)

Figure_1

It can be something like that. I'm not a huge fan of the solution.

Do we want to do that only for plot_vars?

@jobrachem
Copy link
Contributor

I'm not sure whether I correctly understand what the purpose is. Is this interpretation correct?

Arrows that represent input to a variable's distribution are colored in light grey now, to create a visual distinction from arrows that represent input to a variable's calculator. The latter are colored in black, like they used to be.

@GianmarcoCallegher
Copy link
Contributor Author

I'm not sure whether I correctly understand what the purpose is. Is this interpretation correct?

Arrows that represent input to a variable's distribution are colored in light grey now, to create a visual distinction from arrows that represent input to a variable's calculator. The latter are colored in black, like they used to be.

Yes

@jobrachem
Copy link
Contributor

Thanks! Maybe then a legend entry for both cases would be nice, something like "black: Used in value", "grey: Used in distribution". What do you think?

@GianmarcoCallegher
Copy link
Contributor Author

Thanks! Maybe then a legend entry for both cases would be nice, something like "black: Used in value", "grey: Used in distribution". What do you think?

Sure

@GianmarcoCallegher
Copy link
Contributor Author

You can check now

@jobrachem
Copy link
Contributor

image

I like it. One additional question: Why do you write "Used in value or calculator"? Can an input node be used in another node's value without the use of a calculator?

@GianmarcoCallegher
Copy link
Contributor Author

image

I like it. One additional question: Why do you write "Used in value or calculator"? Can an input node be used in another node's value without the use of a calculator?

You are right, it's redundant. I fixed it

@jobrachem jobrachem merged commit b34b9d3 into main Aug 18, 2023
3 checks passed
@jobrachem jobrachem deleted the dist_calc_viz branch August 18, 2023 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:model This issue is related to the model module enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants