Skip to content

A new script to generate a default aenet_input_dir from input.toml. - #86

Open
skasamatsu wants to merge 10 commits into
developfrom
gen_trainer_input
Open

skasamatsu wants to merge 10 commits into
developfrom
gen_trainer_input

Conversation

@skasamatsu

Copy link
Copy Markdown
Contributor

Before, it was up to the user to provide a sensible aenet input file for training and prediction of the on-lattice model. This script automates that process.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

A critical species-ordering issue and additional generation correctness issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds a CLI script that generates default aenet training, prediction, fingerprint, and LAMMPS input files from input.toml.

Changes:

  • Registers the abics_gen_aenet_input command.
  • Generates inputs based on configured species and solver type.
  • Supports output-directory selection and forced overwrites.
File summaries
File Review
pyproject.toml Registers the new CLI entry point.
abics/scripts/gen_aenet_input.py Implements input generation. Species ordering must match the solver, all possible species must be included, multiple training directories must be handled, and forced regeneration must remove stale files.
Review details

Suppressed comments (3)

abics/scripts/gen_aenet_input.py:60

  • This derives the network species from one shuffled config.structure, which only contains species currently selected by the configured counts. It omits valid species with zero initial occupancy (and any species that can later be introduced by a grand-canonical move), so the generated TYPES/NETWORKS and fingerprint files cannot represent all configurations from the input. Build all_species from the base structure plus every non-vacancy group in config.defect_sublattices before applying ignore_species.
    all_species = set(config.structure.symbol_set)
    ignore_species = set(params_root.get("train", {}).get("ignore_species", None) or [])

abics/scripts/gen_aenet_input.py:190

  • TrainerParams supports multiple train.base_input_dir entries (and abics_train requires one generated input set per entry), but this silently selects only the first directory. With a multi-model/ensemble configuration, the command reports success while leaving the other trainer directories absent, so training later fails or uses incomplete inputs; either generate every configured directory or reject lists longer than one instead of discarding them.
        base_input_dir = train_params.get("base_input_dir", "./aenet_train_input")
        if isinstance(base_input_dir, list):
            base_input_dir = base_input_dir[0]
        output_dir = base_input_dir

abics/scripts/gen_aenet_input.py:202

  • --force only permits writing into non-empty directories; it does not remove files that are no longer part of the generated configuration. Re-running after changing the species set or switching solver modes therefore leaves stale fingerprint/network input files in the output tree, so the directory no longer represents the generated default set. Clear or replace each existing output directory when force is requested before regenerating it.
        if os.path.exists(d) and os.listdir(d) and not force:
            print(
                f"Error: {d} already exists and is not empty. Use --force to overwrite.",
                file=sys.stderr,
            )
            sys.exit(1)
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

species_list = " ".join(species)
lines = [
f"pair_style aenet",
f"pair_coeff * * v00 {species_list} {HIDDEN_LAYERS_TAG}.nn {species_list}",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@copilot As written in one of the suppressed comments, the current implementation may not work for grand canonical setup. Please fix it. Also, regarding the assumption of alphabetical ordering, it should be safer for AenetPyLammpsSolver.calculate_energy to look at in.lammps for the generated ordering, I guess.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Addressed in fed4c2d. The generator now derives species from the full configured species set (so grand-canonical cases are covered), aenetPyLammps uses sampling.solver.ignore_species for in.lammps, and AenetPyLammpsSolver now reads the atom-type ordering from in.lammps instead of inferring it from the structure.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Species ordering and ignore-species mismatches can produce incorrect or unusable LAMMPS mappings.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

abics/scripts/gen_aenet_input.py:153

  • This still emits a sorted species list, but the consumer does not use that list to assign LAMMPS atom types: AenetPyLammpsSolver.calculate_energy enumerates st.symbol_set directly (aenet_pylammps.py:75-76), while Input.from_directory only stores the input lines (aenet_pylammps.py:137-142). Since set iteration order need not match this alphabetical order, atom types can be mapped to the wrong aenet species and yield incorrect energies. Please make the solver parse the species order from in.lammps as requested in the previous review, or otherwise share one deterministic mapping between generator and solver.
        f"pair_coeff      * * v00 {species_list} {HIDDEN_LAYERS_TAG}.nn {species_list}",
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

species_list = " ".join(species)
lines = [
f"pair_style aenet",
f"pair_coeff * * v00 {species_list} {HIDDEN_LAYERS_TAG}.nn {species_list}",
Copilot AI and others added 8 commits September 17, 2026 07:01
Co-authored-by: skasamatsu <4555912+skasamatsu@users.noreply.github.com>
Co-authored-by: skasamatsu <4555912+skasamatsu@users.noreply.github.com>
Co-authored-by: skasamatsu <4555912+skasamatsu@users.noreply.github.com>
Co-authored-by: skasamatsu <4555912+skasamatsu@users.noreply.github.com>
Co-authored-by: skasamatsu <4555912+skasamatsu@users.noreply.github.com>
Co-authored-by: skasamatsu <4555912+skasamatsu@users.noreply.github.com>
Co-authored-by: skasamatsu <4555912+skasamatsu@users.noreply.github.com>
Co-authored-by: skasamatsu <4555912+skasamatsu@users.noreply.github.com>
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