Skip to content

fix : ABACUS uses more FFT grids than QE proposed#614

Merged
caic99 merged 2 commits into
deepmodeling:developfrom
Qianruipku:hotfix
Jan 10, 2022
Merged

fix : ABACUS uses more FFT grids than QE proposed#614
caic99 merged 2 commits into
deepmodeling:developfrom
Qianruipku:hotfix

Conversation

@Qianruipku
Copy link
Copy Markdown
Collaborator

@Qianruipku Qianruipku commented Jan 9, 2022 •
edited
Loading

fix a bug that ABACUS uses more FFT grids than QE
some results:
104_PW_AF_magnetic, 104_PW_FM_magnetic, 104_PW_NC_magnetic:
before change: 161616; after change: 151515; QE: 151515
105_PW_FD_smearing, 105_PW_GA_smearing:
before change: 1624100; after change: 1524100; QE: 1524100
150_PW_15_CR_VDW3:
before change: 272790; after change: 252590; QE: 252590
803_PW_LT_bcc:
before change: 303030; after change: 272727; QE: 272727
Thus,result.ref of these are changed.
scope : pw_complement.cpp

@Qianruipku
fix : fix a bug that ABACUS uses more FFT grids than QE proposed in i…
726ead8 
…ssue deepmodeling#613

839_NO_GO_LT_bco
some results:
104_PW_AF_magnetic, 104_PW_FM_magnetic, 104_PW_NC_magnetic:
before change: 16*16*16; after change: 15*15*15; QE: 15*15*15
105_PW_FD_smearing, 105_PW_GA_smearing:
before change: 16*24*100; after change: 15*24*100; QE: 15*24*100
150_PW_15_CR_VDW3:
before change: 27*27*90; after change: 25*25*90; QE: 25*25*90
803_PW_LT_bcc:
before change: 30*30*30; after change: 27*27*27; QE: 27*27*27
Thus,result.ref of these are changed.
scope : pw_complement.cpp
@caic99 caic99 changed the title fix : fix a bug of FFT grids #613 fix : ABACUS uses more FFT grids than QE proposed Jan 10, 2022
@caic99 caic99 linked an issue Jan 10, 2022 that may be closed by this pull request
Bug in calculating fft grids #613
Closed
@caic99
Copy link
Copy Markdown
Member

caic99 commented Jan 10, 2022

@Qianruipku Thank you for your PR!
I suggest you may use a more detailed PR title, just like the title of your commit message. And the detailed info could be provided in PR content, your commit message is just perfect.
You can also link an issue in the 'linked issue' part of PR page. And I've done the things above for you.

caic99
caic99 requested changes Jan 10, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@caic99 caic99 left a comment

Choose a reason for hiding this comment

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

I suggest removing test/build folder if you add it by mistake.

@Qianruipku
Copy link
Copy Markdown
Collaborator Author

Qianruipku commented Jan 10, 2022

Ok, I know how to link an issue now.
test/build is set up by mistake, but I wonder why I cannot see test/build in "Files changed"

@Qianruipku Qianruipku closed this Jan 10, 2022
@Qianruipku Qianruipku deleted the hotfix branch January 10, 2022 08:47
@caic99
Copy link
Copy Markdown
Member

caic99 commented Jan 10, 2022 •
edited
Loading

Ok, I know how to link an issue now. test/build is set up by mistake, but I wonder why I cannot see test/build in "Files changed"

I'm able to see your changes in your commit. Are you browsing your PR after your fix?

@Qianruipku
Copy link
Copy Markdown
Collaborator Author

Qianruipku commented Jan 10, 2022 •
edited
Loading

Oh no! I delete hotfix by mistake！I will re-open it.

@Qianruipku Qianruipku restored the hotfix branch January 10, 2022 09:02
@Qianruipku Qianruipku reopened this Jan 10, 2022
@caic99 caic99 merged commit 4ae1348 into deepmodeling:develop Jan 10, 2022
@Qianruipku Qianruipku deleted the hotfix branch January 10, 2022 12:59
@Qianruipku Qianruipku mentioned this pull request Jan 12, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@caic99 caic99 caic99 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug in calculating fft grids

2 participants

@Qianruipku @caic99