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

Bug with CONVERT_TO_FES and periodic variables #441

Closed
invemichele opened this issue Feb 8, 2019 · 4 comments
Closed

Bug with CONVERT_TO_FES and periodic variables #441

invemichele opened this issue Feb 8, 2019 · 4 comments
Assignees
Labels
Milestone

Comments

@invemichele
Copy link
Contributor

Hi,
together with @luigibonati we realized that CONVERT_TO_FES has a weird small bug when dealing with periodic variables. It looks like it does not consider periodicity and thus adds an extra point compared to the histogram grid. The weird thing is that it does it only for certain values of the GRID_BIN option in the histogram.
When dealing with a 1D FES is not a big deal, there is just one extra point, but for 2D it messes up the whole FES.

To reproduce the bug one can use an alanine COLVAR and analyze it building a histogram of phi with GRID_BIN=300 and then use CONVERT_TO_FES, dumping both the histogram and the fes.
The histogram looks fine, while the fes has an extra point at the end, equal to zero.
The files looks like this:

==> histogram.data <==
#! FIELDS phi his dhis_phi
#! SET normalisation 11120.000000
#! SET min_phi -pi
#! SET max_phi pi
#! SET nbins_phi 300
#! SET periodic_phi true
-3.141593 0.004692 0.102699
-3.120649 0.007288 0.146267
[...]
3.099705 0.001775 0.042773
3.120649 0.002920 0.067943

and

==> fes.data <==
#! FIELDS phi fes dfes_phi
#! SET normalisation 1.000000
#! SET min_phi -pi
#! SET max_phi pi
#! SET nbins_phi 301
#! SET periodic_phi true
-3.141593 13.374476 -54.598476
-3.120718 12.275809 -50.057483
[...]
3.099844 14.557589 -58.043122
3.120718 0.000000 0.000000

if instead we use GRID_BIN=299 or GRID_BIN=100 everything works good. We tried 301 and 150 and they show the bug.

When using a 2D histogram the output of the fes with the bug is messed up. On the left a fes obtained with GRID_BIN=299,299 on the right with GRID_BIN=300,300:
fes

We tried to look at src/gridtools/ConvertToFES.cpp but the code is quite complex for us and we couldn't find the problem.
We can provide all the input files if needed, but everything is pretty standard.
Regards,

Michele and Luigi

@GiovanniBussi
Copy link
Member

@invemichele @luigibonati could you provide an input file? Ideally a fully reproducible input (including all files).

I noticed this problem once with a collaborator that was passing both the spacing and the number of points. If this is also your problem, it would be sufficient to add an error when someone does it. Otherwise there's something we should fix.

Thanks!

@luigibonati
Copy link
Contributor

Hi @GiovanniBussi,
you can find attached a COLVAR file of alanine dipeptide and the input to compute the fes and the histogram, using either 300 or 299 bins. The behaviour is the same if we pass the grid spacing, we did not specify both.

Thank you,
Regards
Luigi and Michele

bug_convert_to_fes.zip

@carlocamilloni
Copy link
Member

I confirm the bug, @gtribello, using CONVERT_FES add additional point to the grid and mishandle the periodicity

@gtribello
Copy link
Member

Hello all. I finally had a chance to look at this. I think it is fixed. If you want to give it another test I would appreciate it. Thanks

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

No branches or pull requests

5 participants