Skip to content

Change GMRF 1D default to Continuous1D from default#61

Open
jakobsj wants to merge 1 commit into
mainfrom
GMRF1D_continuous1D
Open

Change GMRF 1D default to Continuous1D from default#61
jakobsj wants to merge 1 commit into
mainfrom
GMRF1D_continuous1D

Conversation

@jakobsj
Copy link
Copy Markdown
Contributor

@jakobsj jakobsj commented Sep 7, 2022

Fix 1D geometry for GMRF to avoid having just the default geometry. Needed for nb2.

@jakobsj jakobsj added this to the Sprint 11 milestone Sep 7, 2022
@jakobsj jakobsj requested a review from nabriis September 7, 2022 12:07
Copy link
Copy Markdown
Collaborator

@nabriis nabriis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jakobsj ,

Some behavior related to default_geometry will disappear for GMRF if we change this.

For example if a PDEModel uses a StepExpansion for the domain, the user does not have to explicitly add that geometry to a prior because domain geometry of a model takes priority over a defaultGeometry. However with this change the user would have to explicitly add the geometry to the GMRF prior.

I think a better solution is to think of a default geometry for 1D and for 2D (even ND i guess).

In this case if we have e.g. a Wavelet2D on a model domain geometry, the 2D default geometry on the GMRF prior could be allowed to be overwritten (matching behavior of the 1D case)

Hope that makes sense.

@jakobsj
Copy link
Copy Markdown
Contributor Author

jakobsj commented Sep 7, 2022

Right I see the point. This was motivated from wanting to make the point that GMRF automatically has a Continuous1D or Continuous2D assigned based on dom. But in 1D it doesn't currently and in 2D gets Image2D in fact. Maybe better to remove the automatic creation of Image2D as the geometry? Alternatively, could we replace our default geometries by Continuous1D (and Continous2D)?

@jakobsj
Copy link
Copy Markdown
Contributor Author

jakobsj commented Sep 7, 2022

For now maybe easier not to mention GMRF at all in the notebook?

@nabriis
Copy link
Copy Markdown
Collaborator

nabriis commented Sep 7, 2022

Right I see the point. This was motivated from wanting to make the point that GMRF automatically has a Continuous1D or Continuous2D assigned based on dom. But in 1D it doesn't currently and in 2D gets Image2D in fact. Maybe better to remove the automatic creation of Image2D as the geometry? Alternatively, could we replace our default geometries by Continuous1D (and Continous2D)?

From my point of view I see the following user interactions with Geometries as they relate to this discussion.
Point 1. and 2. currently function as written here, while 3. does not (because Image2D is not a DefaultGeometry)

  1. Defining and sampling a 2D GMRF:
    This shows that we need a default choice of geometry that matches the requested physical dimension from the user.
x = GMRF(np.zeros(n), 1, physical_dim=2)
x.sample(5).plot() #User expects to see 2D plots because GMRF is defined with physical_dim=2
  1. Defining distribution in for a parameters to a Model.
    This shows that the default geometry should be allowed to be overwritten.
# Assume we have a PDEModel with variable name model and domain geometry FEniCSContinuous or StepExpansion etc.
# We sample a simple posterior problem as follows:
x = GMRF(np.zeros(n), 1) # Or Gaussian or Uniform etc.
y = Gaussian(model(x), 1)
J = JointDistribution(y,x)
P = J(y=y_obs)
samples = NUTS(P).sample(200,100)

# User would expect the samples to be visualized as FEniCSContinuous or StepExpansion -type plots.
samples["x"].funvals.plot() 
  1. Defining distribution in for a parameters to a Model (in 2D)
    This is the same as point 2. but with a 2D domain geometry
# Assume we have a PDEModel with variable name model and domain geometry FEniCSMesh2D or Continuous2D
# We sample a simple posterior problem as follows:
x = GMRF(np.zeros(n), 1, physical_dim=2) # Or Gaussian (without physical_dim)
y = Gaussian(model(x), 1)
J = JointDistribution(y,x)
P = J(y=y_obs) # Code fails here with error:  geometry of x (Image2D) does not match domain geometry of model (Continuous2D)

# The user would expect this to work similar to the above case, namely that if they do not specify a geometry for a distribution, the domain geometry from the model would be used.

These cases show they we need a dimension aware (1D, 2D etc.) default geometry, which has a default type visualization, but can be overwritten by geometries that are actually specified and match the dimension.

@nabriis
Copy link
Copy Markdown
Collaborator

nabriis commented Sep 7, 2022

For now maybe easier not to mention GMRF at all in the notebook?

@jakobsj, Yes for sure easier not to mention it, but it would be nice to actually have this feature.

@jakobsj
Copy link
Copy Markdown
Contributor Author

jakobsj commented Sep 7, 2022 via email

@nabriis
Copy link
Copy Markdown
Collaborator

nabriis commented Sep 7, 2022

In the interest of time I will remove GMRF for now until this has been resolved. Could Continous1D and 2D etc replace default geometry to achieve the desired result? Default geometry pretty much behaves like Continuous1D anyway?

We explicitly use DefaultGeometry instead to assign it a "lower priority". We need to know if the user explicitly chose Continous1D for a prior and not the default.

@jakobsj
Copy link
Copy Markdown
Contributor Author

jakobsj commented Sep 7, 2022 via email

@nabriis
Copy link
Copy Markdown
Collaborator

nabriis commented Sep 7, 2022

Why? What is achieved by distinguishing Continuous1D from the default?

It allows us to detect easily in the code if the user set a geometry or the geometry was set as default (that is user did not choose it). If the user explicitly sets a geometry we will never overwrite it, but if its a default we can overwrite when it makes sense.

For example I would expect this to pass:

# Assume we have a PDEModel with variable name model and domain geometry FEniCSContinuous or StepExpansion etc.
x = GMRF(np.zeros(n), 1) # Or Gaussian or Uniform etc.
y = Gaussian(model(x), 1)
J = JointDistribution(y,x)
P = J(y=y_obs)
samples = NUTS(P).sample(200,100)
samples["x"].funvals.plot() 
# User would expect the samples to be visualized as FEniCSContinuous or StepExpansion -type plots.

But this to fail:

# Assume we have a PDEModel with variable name model and domain geometry FEniCSContinuous or StepExpansion etc.
x = GMRF(np.zeros(n), 1, geometry=Continuous1D(n))  # Or Gaussian or Uniform etc.
y = Gaussian(model(x), 1)
J = JointDistribution(y,x)
P = J(y=y_obs) # Should fail because model domain geometry does not match x geometry.

@jakobsj
Copy link
Copy Markdown
Contributor Author

jakobsj commented Sep 7, 2022 via email

@nabriis
Copy link
Copy Markdown
Collaborator

nabriis commented Sep 7, 2022 via email

@nabriis nabriis removed this from the Sprint 11 milestone Sep 7, 2022
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.

3 participants