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

Define quantities only once #118

Merged
merged 2 commits into from
Dec 11, 2023
Merged

Define quantities only once #118

merged 2 commits into from
Dec 11, 2023

Conversation

jan-janssen
Copy link
Member

Validate using:

from atomistics.calculators.lammps import calc_molecular_dynamics_nvt_with_lammps

calc_molecular_dynamics_nvt_with_lammps?

@samwaseda
Copy link
Member

It's already going in the good direction, but I think we should totally get rid of the independent definitions of the tags and the functions. I don't particularly like my suggestion below, but it might give you the direction that I would like to take:

class EngineNotFound:
    def __call__(self):
        raise ValueError("engine not defined")

class EngineContainer:
    def __getattribute__(self, attr):
        return EngineNotFound()

def get_interactive_getter(engine=EngineContainer()):
    return {
        "positions": engine.interactive_positions_getter,
        "cell": engine.interactive_cells_getter,
        "forces": engine.interactive_forces_getter,
        "temperature": engine.interactive_temperatures_getter,
        "energy_pot": engine.interactive_energy_pot_getter,
        "energy_tot": engine.interactive_energy_tot_getter,
        "pressure": engine.interactive_pressures_getter,
        "velocities": engine.interactive_velocities_getter,
    }
print(list(get_interactive_getter().keys()))

Output: ['positions', 'cell', 'forces', 'temperature', 'energy_pot', 'energy_tot', 'pressure', 'velocities']

get_interactive_getter()["positions"]()

Output: ValueError: engine not defined

And this would replace the interactive_getter_dict in helper.py.

@samwaseda samwaseda changed the title Define quantities only ones Define quantities only once Dec 11, 2023
@liamhuber
Copy link
Member

It's already going in the good direction, but I think we should totally get rid of the independent definitions of the tags and the functions. I don't particularly like my suggestion below, but it might give you the direction that I would like to take:

Agreed, this is the direction although the implementation still needs some TLC.

@jan-janssen
Copy link
Member Author

To be able to keep the momentum in the development, I am going to move ahead and merge this pull request and then we can look into general changes in a separate pull request.

@jan-janssen jan-janssen merged commit 7574c93 into main Dec 11, 2023
17 checks passed
@jan-janssen jan-janssen deleted the quantities branch December 11, 2023 19:23
@liamhuber liamhuber mentioned this pull request Dec 11, 2023
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

Successfully merging this pull request may close these issues.

3 participants