Add toggle for new emmet core features

[esoteric-ephemera]

From atomate2 #1325, it was suggested that new emmet features, like models which replace pymatgen ones for bandstructure, DOS, etc., be an opt-in feature

Adds basic toggle here for the emmet models EmmetSettings.USE_EMMET_MODELS to avoid a generic "TOGGLE_DEV_FEATURES" kind of thing

[codecov-commenter]

Codecov Report

❌ Patch coverage is 77.02703% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.10%. Comparing base (013ead9) to head (98831c5).

Files with missing lines	Patch %	Lines
emmet-core/emmet/core/trajectory.py	72.72%	9 Missing ⚠️
emmet-core/emmet/core/vasp/calculation.py	68.00%	8 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1348      +/-   ##
===========================================
+ Coverage    86.03%   86.10%   +0.07%     
===========================================
  Files          227      227              
  Lines        17791    17827      +36     
===========================================
+ Hits         15306    15350      +44     
+ Misses        2485     2477       -8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tsmathis]

saw the initial review request and looked briefly, only thing I'll say is that having the default be True makes the emmet models choice opt-out, rather than opt-in, correct?

[esoteric-ephemera]

Sorry forgot I already sent a request! Default, USE_EMMET_MODELS = True, is opt-in for the emmet models

One other thing I noticed: the Trajectory classes try to ensure that the ordering of elements is consistent across all frames. That re-ordering didn't get applied to the properties which are tagged to a site, the forces and magmoms

AFAIK, the site/element order is always consistent across ionic steps in a vasprun / task doc, so that doesn't matter for the current trajectory collection. I can check to make doubly sure tho

[tsmathis]

Sorry forgot I already sent a request! Default, USE_EMMET_MODELS = True, is opt-in for the emmet models

Sorry, should have been clearer, by having the flag be True by default in EmmetSettings there is an implicit opt-in at runtime happening, and users would have to explicitly opt-out in their code?

[tsmathis]

AFAIK, the site/element order is always consistent across ionic steps in a vasprun / task doc, so that doesn't matter for the current trajectory collection. I can check to make doubly sure tho

If you find we need to re-generate the trajectory parquet dataset its nbd 👍

[esoteric-ephemera]

Sorry, should have been clearer, by having the flag be True by default in EmmetSettings there is an implicit opt-in at runtime happening, and users would have to explicitly opt-out in their code?

Correct but happy to change if we want to switch to opt-in pattern moving forwards

[esoteric-ephemera]

Default is now opt-in for the new features - we don't seem to test the vasp objects stored in emmet so the setting doesn't matter for test purposes now

We should probably add tests for toggling on/off specifically, will work on that

[tsmathis]

sounds good, just tag us when you want review or think it's ready

[esoteric-ephemera]

Should be good to go!

[tsmathis]

nothing to add overall, looks good 👍