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

Warn user when specifying invalid kwargs to equilibrium and calculate #52

Closed
richardotis opened this issue Aug 11, 2016 · 8 comments
Closed

Comments

@richardotis
Copy link
Collaborator

Example: when specifying raw pdens as kwarg to equilibrium() -- this is incorrect

@bocklund
Copy link
Collaborator

Can we just add a check that if len(kwargs) != 0: warn and print(kwargs) to the end of equilibrium (and calculate, etc.)? The idea being since we use kwargs.pop() to get our kwargs, ideally kwargs is empty at the end of the function since we've popped everything we want and if len(kwargs) is non-zero that means we passed some kwarg that did not get used.

One downside, besides being messy, is that kwargs in branch logic that don't get popped would raise warnings. Not sure where/if that would happen.

It seems like someone else should've solved this problem nicely, but I'm not using the right search terms for it.

@richardotis
Copy link
Collaborator Author

Your solution sounds fine.

@bocklund
Copy link
Collaborator

Which functions? equilibrium and calculate? Since those are the ones that would affect the intended calculation. Anywhere else?

@richardotis
Copy link
Collaborator Author

I'm inclined to say just equilibrium and calculate since the API for other pycalphad objects isn't too complicated. One issue is that this is liable to cause forward and backward compatibility issues as kwargs appear and disappear from the API.

@bocklund
Copy link
Collaborator

Regarding compatibility: that's what we want right? You don't want equilibrium/calculate to eat some outdated API and not give the intended result.

@richardotis
Copy link
Collaborator Author

richardotis commented Apr 19, 2017 via email

@bocklund
Copy link
Collaborator

calculate tries to convert remaining kwargs into StateVariables, so passing invalid kwargs to calculate is actually not possible without raising an exception (line 415 in calculate.py).

@richardotis
Copy link
Collaborator Author

Just handling the case for equilibrium should be fine then.

bocklund added a commit to bocklund/pycalphad that referenced this issue Aug 17, 2021
Fit single phase data. Data is weighted based on what type of quantity it is and directly added to the phase equilibria error.

* WIP: initial sketch of single phase error
* WIP: Use constant weight factors in single phase error
* ENH: Integrate single phase error into lnprob
* Explictly use delayed database and phase models for multi phase mcmc
* Handle null case in finding optimal parameters.
This is a stopgap for now, it should be investigated why this is happening.
* Integrate calculate_single_phase_error into existing tests
* MAINT: maintain tests for delayed_dbf
* TST: Write test for single phase lnprob error
* ENH: use ravel_conditions method in single phase error calculation
* Remove cache
* MAINT: Remove 'delayed' code
* MAINT: Fix typo
* TST: Remove delayed from tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants