ColwynGulliford / lume-gpt

Apache License 2.0
3 stars 4 forks source link

Does not work with new distgen "Prepping for 2.0" #13

Closed petr-lanl closed 5 months ago

petr-lanl commented 5 months ago

distgen/generator.py line 209 requires a new param ('species') in settings file since 3 weeks ago: required_params = ['n_particle', 'total_charge', 'species']

run_gpt_with_settings() from GPT_tools/GPTExtension.py but I think lume-gpt skips 'species': Skipping param species

I am not sure which other functions in lume-gpt would skip "species" parameter or whether I have to go back to GPT_tools/GPTExtension.py to search for answers.

I am currently removing "species" from the required parameters and I get:

[distgen/generator.py:267](python3.11/site-packages/distgen/generator.py#line=266): UserWarning: Particle species not set, defaulting to species = "electron"
  warnings.warn('Particle species not set, defaulting to species = "electron"')
ColwynGulliford commented 5 months ago

Thanks for submitting this issue. I checked, and the Lume-GPT function does at least work as intended with Distgen 2.0. The keyword "species" should appear in the distgen file itself, not in the settings dictionary passed in:

run_gpt_with_distgen(settings, distgen_input_file=path_to_your_distgen_file)

Can you tryin adding the species key to the distgen file you are using?

Also, while you can if you want in your own copy, changing the Distgen code itself is not recommended outside of testing/debugging - Distgen now has many tests to make sure it behaves correctly and changing the code could result in unexpected results.

petr-lanl commented 5 months ago

Thank you for clarification. I thought that adding species to the settings would be sufficient.

ColwynGulliford commented 5 months ago

Of course, it's good you raised the issue as the examples in lume-gpt with distgen files had not been modified to include the species key. I've fixed that.