{"action":"created","issue":{"url":"https://api.github.com/repos/pandegroup/openmm/issues/1910","repository_url":"https://api.github.com/repos/pandegroup/openmm","labels_url":"https://api.github.com/repos/pandegroup/openmm/issues/1910/labels{/name}","comments_url":"https://api.github.com/repos/pandegroup/openmm/issues/1910/comments","events_url":"https://api.github.com/repos/pandegroup/openmm/issues/1910/events","html_url":"https://github.com/pandegroup/openmm/issues/1910","id":262033267,"number":1910,"title":"Using a barostat ignores enforcePeriodicBox","user":{"login":"stefdoerr","id":7935362,"avatar_url":"https://avatars3.githubusercontent.com/u/7935362?v=4","gravatar_id":"","url":"https://api.github.com/users/stefdoerr","html_url":"https://github.com/stefdoerr","followers_url":"https://api.github.com/users/stefdoerr/followers","following_url":"https://api.github.com/users/stefdoerr/following{/other_user}","gists_url":"https://api.github.com/users/stefdoerr/gists{/gist_id}","starred_url":"https://api.github.com/users/stefdoerr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/stefdoerr/subscriptions","organizations_url":"https://api.github.com/users/stefdoerr/orgs","repos_url":"https://api.github.com/users/stefdoerr/repos","events_url":"https://api.github.com/users/stefdoerr/events{/privacy}","received_events_url":"https://api.github.com/users/stefdoerr/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":11,"created_at":"2017-10-02T09:50:22Z","updated_at":"2017-10-04T22:02:07Z","closed_at":"2017-10-02T11:43:29Z","author_association":"CONTRIBUTOR","body":"So the documentation of getState shows a flag for disabling the wrapping:\r\nhttp://docs.openmm.org/6.3.0/api-c++/classOpenMM_1_1Context.html#a5c53803407b6c22454813f668c42a33e\r\n\r\nI noticed that if I simulate my system with a barostat I get wrapped coordinates.\r\n```C++\r\nOpenMM::Force *barostat = new OpenMM::MonteCarloBarostat(sim->barostatpressure, sim->thermostattemp);\r\nsystem->addForce(barostat);\r\n[...]\r\nOpenMM::State state = sim->context->getState( OpenMM::State::Positions | OpenMM::State::Energy, false );\r\n```\r\nIf I don't add the barostat I get them unwrapped. Is this expected behaviour?"},"comment":{"url":"https://api.github.com/repos/pandegroup/openmm/issues/comments/334302591","html_url":"https://github.com/pandegroup/openmm/issues/1910#issuecomment-334302591","issue_url":"https://api.github.com/repos/pandegroup/openmm/issues/1910","id":334302591,"user":{"login":"peastman","id":4379786,"avatar_url":"https://avatars3.githubusercontent.com/u/4379786?v=4","gravatar_id":"","url":"https://api.github.com/users/peastman","html_url":"https://github.com/peastman","followers_url":"https://api.github.com/users/peastman/followers","following_url":"https://api.github.com/users/peastman/following{/other_user}","gists_url":"https://api.github.com/users/peastman/gists{/gist_id}","starred_url":"https://api.github.com/users/peastman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/peastman/subscriptions","organizations_url":"https://api.github.com/users/peastman/orgs","repos_url":"https://api.github.com/users/peastman/repos","events_url":"https://api.github.com/users/peastman/events{/privacy}","received_events_url":"https://api.github.com/users/peastman/received_events","type":"User","site_admin":false},"created_at":"2017-10-04T22:02:07Z","updated_at":"2017-10-04T22:02:07Z","author_association":"OWNER","body":"The `ApplyMonteCarloBarostatKernel` class has a method called `scaleCoordinates()`:\r\n\r\nhttps://github.com/pandegroup/openmm/blob/master/olla/include/openmm/kernels.h#L1266-L1278\r\n\r\nEach platform has a subclass that provides an implementation of that method, which is where this happens.  For example, here's where it happens for the reference platform:\r\n\r\nhttps://github.com/pandegroup/openmm/blob/master/platforms/reference/src/SimTKReference/ReferenceMonteCarloBarostat.cpp#L85-L90\r\n\r\nThe CUDA version is a bit more complicated, since there's both host and device code involved.  Here's the kernel code that moves each molecule into the first periodic box:\r\n\r\nhttps://github.com/pandegroup/openmm/blob/master/platforms/cuda/src/kernels/monteCarloBarostat.cu#L27-L31\r\n\r\nBut in addition to that, it stores in host memory (*not* device memory) a set of integers for how many box widths each particle has been translated by.  To preserve the unwrapped positions, it would need to record how many box widths the kernel moved each particle by, send that information back to the host, and update the counts there (but only if the move is accepted).  Currently we don't try to do that.  We just set all the counts to 0:\r\n\r\nhttps://github.com/pandegroup/openmm/blob/master/platforms/cuda/src/CudaKernels.cpp#L8195-L8196"}}
{"actor":{"display_login":"peastman"}}