-
Notifications
You must be signed in to change notification settings - Fork 681
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
Allow LammpsDumpParser Topology reader more flexibility in coordinate column data. #3449
Comments
@hmacdope I un-assigned you from this issue, otherwise GSoC Contributors will wonder if they can pick the issue to work on. |
Okay all good 👍 |
Hello, I'm RIya Elizabeth John, an Outreachy applicant. Could you provide some more context about the |
The LAMMPs parser is in this file. Currently it expects |
Apologies for the late response, I had been travelling on our class trip. |
@Riyabelle25 sorry should have been clearer, you just need to change the logic in the existing class :) |
I want to contribute. |
Hey! I'm interested in contributing to this issue, I would have one question, when talking about: "we should relax the requirement for an id, type, x, y, z format and make it somewhat more flexible." You mean deleting totally/some fields of these lines?:
or what exactly is meant by: "more flexible". Thank you so much in advance! |
Hi! I am an MDAnalysis user and have just started using lammps. I would absolutely use this functionality, especially if it were general (for example, I have been needing to hack around to read forces using MDAnalysis). I don't think I could contribute to changing the parser, but let me know if I can help out in any other way (providing example dump files with different properties, testing things out). |
@aliehlen any kind of help is appreciated. If you are a user of LAMMPS then please say how MDAnalysis could work better for you. Issues should focus on one specific thing. You can open new issues or starting a discussion on the developer mailing list. Examples of input files (maybe a snippet in a comment for discussion) and links to format specifications that you know to be relevant are useful. For example, I know very little about LAMMPS (so I also can't say much about #3449 (comment) ) but I can judge an approach if someone clearly explains
The last point is not necessary to get a discussion started. Initially, it's more important to understand what the problem is and what a solution should provide. |
It means being able to parse data under more columns than "Id type ..." and also have them possibly out of order. 😀 |
Hey! I wanna work on this issue, but I got confused, so I decided to ask a few questions before moving forward:
|
@amirhs1 for the DataParser, you can provide the The aim of this issue is to do something similar for the DumpParser. Have a read of the source code in https://github.com/MDAnalysis/mdanalysis/blob/develop/package/MDAnalysis/topology/LAMMPSParser.py which already has a lot of the functionality you will need. |
I created a branch called "issue3449-LammpsDumpParser" from "develop" branch. |
It will be on your own fork of MDAnalysis. :)
On Sun, 17 Apr 2022 at 9:45 pm, amirhsi ***@***.***> wrote:
I create a branch called "issue3449-LammpsDumpParser" from "develop"
branch. I can see it on my side on, but I cannot find it on MDAnalysis
github.
—
Reply to this email directly, view it on GitHub
<#3449 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AF3RHC74DR4B6JVS7CGUFA3VFP2W3ANCNFSM5HBUGMCA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
Hugo MacDermott-Opeskin
Post Doctoral Fellow, RSC ANU
|
Hello @hmacdope, I'm Fortune from the Outreachy program and am interested in contributing to this project. I think I understand the issue but I have just one question about the intended According to the LAMMPS docs for the dump command, these are the possible attributes for
Should all be parsed and passed to their respective TopologyAttrs subclass? The |
Sorry for the delay (I was away). I would open an issue about this but it seems that there are a few open already (this one, issue #3504 ) and a few PRs to deal with them (PR #3448 , #3608 , #3647 ). So I'd just like to voice support for this one and #3504 :). As is discussed here, lammps dump files are very flexible and customizable and it would be great to be able to read in other properties. Even though I'm not an extremely experienced user of lammps or MDAnalysis, I've already run into situations in which I want to read in properties from a dump file that can't currently be read in (I've attached an example of a dump file that has a simple custom style). Reading through these comments, I think PR #3647 has allowed for the topology parser to accept different atom_styles. I want to point out that it also (seemly) is common to have custom dump formats, so it would be great if those can be handled by the topology parser, as well (or, at least, they shouldn't break the parser). PR #3608 deals with arbitrary columns for reading in coordinate data in a really general way---I'm not sure any of that could apply to the topology parsing, though. Anyway, I don't know if there's a good way to help with these two specific PRs, but I can test out different files and configurations here if needed! |
Hello, I want to ask a question concerning what you meant on making the |
Hi @hmacdope, I received the invitation to submit a PR for the purpose of applying to gsoc'24. I'm interested in contributing to this issue. Having gone through the discussions above, I understand that I need to allow more flexibility to user in the passing o their dup file. Please is this definition ok? If so I'll:
If this approach is fine with you, please assign it to me |
|
Hello @vdjeudjam we normally don't assign issues for GSOC or other programs. Just submit a PR that references the issue. We will review the first one. |
Is your feature request related to a problem?
The LAMMPS DumpParser topology creator is somewhat inflexible in the column format it expects.
Now that we support other coordinate conventions #3360 and velocities #3448 (WIP), we should relax the requirement for an
format and make it somewhat more flexible.
Describe the solution you'd like
Allow the parser more flexibility in available column data.
Describe alternatives you've considered
Additional context
The text was updated successfully, but these errors were encountered: