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

Expand and revamp model and symbol constraints #215

Open
clegaspi opened this issue Mar 22, 2019 · 2 comments
Open

Expand and revamp model and symbol constraints #215

clegaspi opened this issue Mar 22, 2019 · 2 comments

Comments

@clegaspi
Copy link
Contributor

clegaspi commented Mar 22, 2019

As it is put in some of the code comments, constraints are a bit "hacked together" at the moment. :)

Constraints should probably be set apart as their own class, instead of being a subclass of Model. I can see it was done initially for the sympy parsing and plug in, but models have much more functionality and metadata that constraints don't need or need differently. Maybe there just needs to be more overridden functions. I haven't taken a very close look at it, so it's hard to say right now what is the best course of action.

Also, current implementation does not really support constraints that involve non-numeric quantities/symbols. For example, if a model only applies to non-magnetic materials, we should have a constraint magnetic_order == "NM". At present, sympy can kind of figure this out, but not really. A thought would be to generate lambda functions for these kinds of expressions or to use eval() and warn users to not import any YAML files from untrusted sources.

@maxhutch
Copy link

maxhutch commented Apr 3, 2019

Do you have any concrete examples of constraints that aren't independent (i.e. rectangular)? For example, not isMetal and resistivity > 1.0e-3 and refractive_index > 1.5 would be independent but resistivity * refractive_index > 1.5e-3 would not be.

@clegaspi
Copy link
Contributor Author

clegaspi commented Apr 3, 2019

Currently, no. Although the current implementation should work for dependent (non-independent?) constraints.

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

No branches or pull requests

2 participants