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

memory not allocated when customColvar is used as a cvc with vector-valued sub-cvcs? #569

Closed
zwpku opened this issue Aug 28, 2023 · 2 comments · Fixed by #571
Closed

memory not allocated when customColvar is used as a cvc with vector-valued sub-cvcs? #569

zwpku opened this issue Aug 28, 2023 · 2 comments · Fixed by #571
Labels
minor bug technically a bug, does not affect user experience in real life

Comments

@zwpku
Copy link
Member

zwpku commented Aug 28, 2023

Hi,
I just report a minor issue/bug on the definition of the variable cv_force through the line
colvarvalue cv_force(current_cv_value.type());
in the function void colvar::customColvar::apply_force(colvarvalue const &force) located in the file colvarcomp_combination.cpp.

It seems that this definition does not allocate memory when a sub-cvc is vector-valued. This may lead to segmentation fault, which can be recovered with a CV defined as follows:

colvar {
  name nn_0
  customFunction z
  lowerBoundary -2.5
  upperBoundary 3.2
  width 0.05
  extendedLagrangian on
  extendedFluctuation 0.1
  extendedTimeConstant 200

  customColvar {
    name z
    customFunction x1+y2
    cartesian {
      name x
      atoms { 
    atomNumbers {
      1 
    }
      }
    }
    cartesian {
      name y
      atoms { 
    atomNumbers {
      2  
    }
      }
    }
  }
}

This issue could be resolved by replacing the definition of cv_force with:

colvarvalue cv_force(current_cv_value);
cv_force.reset();
@zwpku zwpku added the minor bug technically a bug, does not affect user experience in real life label Aug 28, 2023
@HanatoK
Copy link
Member

HanatoK commented Aug 28, 2023

@zwpku Thanks for reporting the bug! I thought the two ways of construction, colvarvalue(Type const &vti) and colvarvalue(colvarvalue const &x); reset() should behave the same, but to my surprise they don't, as the type information of colvarvalue does not contain the size of the vector. Could you open a PR?

Update:
Your PR just reminds me another constraint of using nested CVCs in linearCombination, so I open a PR to fix both.

@giacomofiorin
Copy link
Member

I agree that the behavior should be better documented, specifically to remind that the Type enum alone is not sufficient for variables that are vectors of arbitrary length. I added a commit to PR #571 to that sense.

HanatoK added a commit to HanatoK/colvars that referenced this issue Aug 30, 2023
@zwpku zwpku closed this as completed Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor bug technically a bug, does not affect user experience in real life
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants