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

CellVariable hasOld() should set self.old #77

Closed
wd15 opened this issue Sep 19, 2014 · 2 comments
Closed

CellVariable hasOld() should set self.old #77

wd15 opened this issue Sep 19, 2014 · 2 comments

Comments

@wd15
Copy link
Contributor

wd15 commented Sep 19, 2014

CellVariable hasOld() should set self.old=self.copy() if self.old is None and hasOld was not explicitly set to False in __init__(). This avoids the problem of forgetting to set hasOld=True when using sweep() and then sweep() just iterating forward as if it was a full time step.

Imported from trac ticket #85, created by wd15 on 09-19-2006 at 14:51, last modified: 06-21-2011 at 13:19

@wd15
Copy link
Contributor Author

wd15 commented Sep 19, 2014

I think I mean updateOld() rather than hasOld().

Trac comment by wd15 on 06-20-2011 at 16:34

@wd15
Copy link
Contributor Author

wd15 commented Sep 19, 2014

Setting self._old to be self.copy() in updateOld() leads to some unintended consequences. One possible scenario is this

  • hasOld is False on instantiation
  • the variable assigns constraints
  • an explicit diffusion term is used
  • updateOld is then called

The equation will use the old value without the constraints and the constraints won't get seen by the diffusion term (which uses the old value for explicit cases). Changing what var.old is defined as is quite dangerous in this respect and in others. A better option is to just throw an exception if updateOld() is called when hasOld has not been set to True. This is done in r4632.

Trac comment by wd15 on 06-21-2011 at 13:19

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants