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

Hackathon TODO list #55

Closed
7 of 12 tasks
jameskermode opened this issue Mar 16, 2017 · 13 comments
Closed
7 of 12 tasks

Hackathon TODO list #55

jameskermode opened this issue Mar 16, 2017 · 13 comments
Assignees

Comments

@jameskermode
Copy link
Member

jameskermode commented Mar 16, 2017

QUIP Hackathon TODO

This issue is to collect tasks for next week's QUIP Hackathon. Please edit the list below, adding links to other issues where appropriate. We can then add tags to assign tasks to people who would like to work on them.

Code Maintenance

Documentation

New features

@jameskermode
Copy link
Member Author

jameskermode commented Mar 23, 2017

We agreed to deprecate FortranArray by adding warnings whenever accessed

@albapa
Copy link
Member

albapa commented Mar 23, 2017

How will atoms/neighbours be indexed in quippy then? Still one-based? It used to be quite convenient to loop over them with frange(at.n).

@jameskermode
Copy link
Member Author

jameskermode commented Mar 23, 2017

For ASE-style usage such as at.get_positions(), which we will use for all new examples and documentation, you will continue to get a standard numpy array.

For quippy-style usage such asat.pos the FortranArray would still be there but you would get a warning when accessing it. We can then review in ~1 years time and see how many people are using in this way.

@gabor1
Copy link
Contributor

gabor1 commented Mar 23, 2017

And we will be able to turn off the warning system if you want to write quippy style

@albapa
Copy link
Member

albapa commented Mar 23, 2017

Sounds good. The reason I'm asking because I very often use the neighbour() function which expects the arguments as one-based. I'd use ase if the same functionality existed but last time I looked I couldn't find it.

@albapa
Copy link
Member

albapa commented Mar 23, 2017

Is anyone working on the scaling factors for the Sum potential?

@gabor1
Copy link
Contributor

gabor1 commented Mar 23, 2017

You can continue to write quippy code, and it will be fine

@gabor1
Copy link
Contributor

gabor1 commented Mar 23, 2017

You can't implement scaling in a generic way, so let's just issue the warning. go ahead @albapa and do it if you like

@jameskermode
Copy link
Member Author

we've just been talking about neighbour lists - ASE builtin is $O(N^2)$ and hence slow. ASE-compatible and fast alternative to QUIP is matscipy.neigbour_list.neighbours.

@albapa
Copy link
Member

albapa commented Mar 23, 2017

regarding the scaling factors - would that be energy and r-scale both?

@jameskermode
Copy link
Member Author

See Gabor's comments under issue #51 for the scaling factors - I think the current task is only to add warnings.

@jameskermode jameskermode self-assigned this Mar 23, 2017
@jameskermode
Copy link
Member Author

If there are no objections I'm going to close this issue - I think everything is recorded on separate issues. I'll make a PR for Hackathon branch now; hackathon-f90wrap will stay around for a while longer as we continue to work on it.

@jameskermode
Copy link
Member Author

FortranArray is no more. ase convert should do most of what convert.py used to; open an issue at ASE if there's missing functionality

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

No branches or pull requests

3 participants