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.

Sign up for GitHub

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

Jump to bottom

Unexpected error when set a float value #51

Closed
JunCEEE opened this issue Feb 17, 2022 · 2 comments
Closed

Unexpected error when set a float value #51

JunCEEE opened this issue Feb 17, 2022 · 2 comments
Assignees
@JunCEEE @shervin86

Comments

@JunCEEE
Copy link
Contributor

JunCEEE commented Feb 17, 2022

When run this piece of code:

diver_value = get_divergence_from_beam_diameter(photon_energy.value, diameter.value)
divergence.value = diver_value

def get_divergence_from_beam_diameter(E, beam_diameter_fwhm):
    """Calculate the divergence (radian) from and photon energy (eV) beam_diameter (m)"""
    # The rms of the amplitude distribution (Gaussian)
    E = Quantity(E, Unit("eV"))
    beam_waist = beam_diameter_fwhm / np.sqrt(2.0 * np.log(2.0))
    theta = 2.0 * hbar * c / beam_waist / E.to("joule").magnitude

    return theta

Got the following error, which is unexpected:

Traceback (most recent call last):
  File "test_GaussianSourceCalculator.py", line 8, in <module>
    calculator = GaussianSourceCalculator("gaussian_source",WPG_path)
  File "/gpfs/exfel/data/user/juncheng/SimExLite/SimExLite/SourceCalculators/GaussianSourceCalculator.py", line 28, in __init__
    super().__init__(
  File "/gpfs/exfel/data/user/juncheng/libpyvinyl/libpyvinyl/BaseCalculator.py", line 127, in __init__
    self.parameters = parameters
  File "/gpfs/exfel/data/user/juncheng/libpyvinyl/libpyvinyl/BaseCalculator.py", line 166, in parameters
    self.set_parameters(value)
  File "/gpfs/exfel/data/user/juncheng/libpyvinyl/libpyvinyl/BaseCalculator.py", line 173, in set_parameters
    self.init_parameters()
  File "/gpfs/exfel/data/user/juncheng/SimExLite/SimExLite/SourceCalculators/GaussianSourceCalculator.py", line 91, in init_parameters
    divergence.value = diver_value
  File "/gpfs/exfel/data/user/juncheng/libpyvinyl/libpyvinyl/Parameters/Parameter.py", line 235, in value
    self.__check_compatibility(value)
  File "/gpfs/exfel/data/user/juncheng/libpyvinyl/libpyvinyl/Parameters/Parameter.py", line 219, in __check_compatibility
    raise TypeError(
TypeError: New value of type <class 'numpy.float64'> is different from <class 'pint.quantity.Quantity'> previously defined

@shervin86 Could you please have a look at it? I guess it's because only float instead of numpy.float64 was accepted. Maybe the solution is to show a more clear instruction in the error or accept numpy types as well.
@JunCEEE
Copy link
Contributor Author

JunCEEE commented Feb 25, 2022

@JunCEEE Add testing in libpyvinyl

JunCEEE added a commit that referenced this issue Feb 25, 2022
@JunCEEE
Add Parameters test corresponding to #51
9bde583
@JunCEEE
Copy link
Contributor Author

JunCEEE commented Feb 25, 2022 •
edited
Loading

@shervin86 tests added to the develop branch. It throws errors now. See: 9bde583

@JunCEEE JunCEEE mentioned this issue Feb 25, 2022
Merged
@JunCEEE JunCEEE closed this as completed Feb 27, 2022
CFGrote added a commit that referenced this issue Feb 28, 2022
@JunCEEE @CFGrote
@mads-bertelsen @shervin86
New release 0.1.0 (#52)
03c6acb 
* Fixing html footer.

* Adding more py versions to be supported.

* Update gitignore

egg files are now ignored.

* Added new ci file for github actions.

* Removed obsolet travis ci file.

* Fixing html footer.

* Adding more py versions to be supported.

* Update gitignore

egg files are now ignored.

* Added new ci file for github actions.

* Removed obsolet travis ci file.

* Carsten (#30)

* Fixing version issue in CI setup.

* Update ci file.

libpyvinyl was not installed into testing environment, this caused ImportError.

* Update gitignor

Ignore emacs swap files.

* Small cosmetic fixes.

Co-authored-by: Carsten Fortmann-Grote <[email protected]>

* Add readthedocs.yaml

Configuration of basic rtd settings now live in this file as recommended by
readthedocs.

* Update readthedocs.yaml

Fixed ubuntu version

* Update readthedocs.yaml

Fixing path to rtd python conf file.

* Update readthedocs.yaml

Add global requirements and project to install step

* Update readthedocs.yaml

Trying to fix package install.

* Remove readthecods.yaml from git.

* Update refman.rst

Replace autoclass by automodule statements.
Include Collection and Instrument modules.

* Update authors.

* Update gitignore.

json dumps, h5 and tmp files produced by examples and tests are now ignored.

* Updated parameters container to Ordered_dict to preserve parameter order.

Moved value from an attribute to a property. In connection to this, I made the consequence of entering an illegal value an error instead of it just being ignored. This was chosen as the setter function is now hidden from the user, so they wouldnt necessarily know a check is being made behind the scences.

Updated tests to account for removal of set_value function.

* using json_tricks in order to support dump and load of numpy arrays in json

* instructions to run tests: suggested pytest package for developers

* more tests for the Parameter class

* - changed logic of interval
 - modified tests to accomodate new logic for intervals
 - add_interval and add_option now have an additional argument

* small fix to ParameterTest

* Suggested changes by Junchen: setup.py taking requirements from requirements.txt file

* Replace interval and option with _interval and _option

* Using closed intervals on both sides

* Fixing logic of is_legal to simplify the reading of the code

* Fixing and adding more tests for checking validity of values

* More meaningful error message when looking for a parameter that does not exist

* Addressing Carsten comments

* Added __iter__ and __next__ methods to CalculatorParameters which
use the underlying parameters.values() __iter__ and __next__ methods.

If we have a CalculatorParameters object called A, these two are eqvivalent:

for parameter in A.parameters.values():
    print(parameter)

for parameter in A:
    print(parameter)

It is also possible to get all the parameter objects contained in
a CalculatorParameters object into a list with just:

parameter_list = list(A)

* set_parameters method in base calculator

* Updated test to check set_parameters.

* removed specialized calculator and removed useless numpy dependency from BaseCalculator

* Specializedcalculator moved into the Tests

* forgotten SpecializedCalculator in tests/

* Split requirements in prod.txt and dev.txt

* github workflow using black also added to the dev requirements

* Update ci.yml

* Fix CI

* reformatted by black

* Several changes:
 - Introduced type checks for values, intervals and options.
 - Using pint.Quantity for floats and physical quantities with units
 - Encode and decode pint.Quantity in json

* Adding test for using units.

* Removed print

* Code cleaning and improvements plus more tests

* respecting flake8 suggestion as reported by Junchen

* fixing typos

* Add BaseData.py

* add DiffractionData examples

* merge

* Update ci file.

libpyvinyl was not installed into testing environment, this caused ImportError.

* Update data and format base classes and example.

In this way, we can avoid dynamic module loading.

* Adding specs

* Update api_spec.py

Tried to combine Jun's and my earlier prototype into one. Not entirely happy
yet, need to add parameters / collections to get the whole story.

* add pseudocode

* add pseudocode

* modified based on the first interation of reviews.

* update BaseData and BaseFormat examples

* Add module docstring.

* comply snake case naming style

* cleaned up the pseudo code for discussion

* nice examples

* add BaseDataTest.py

* clean up unnecessary test and add integration test

* NumberCalculatorsFinished

* update PlusCalculator

* Add test_ArrayCalculators.py

* add test_Instrument.py

* Update BaseCalculator in plusminus

* the new proposed BaseCalculator

* No need input_keys for calculator init

* Format: Predicble return object from write

* add initialization check

* minor: error raise info updated

* fix errors for test_ArrayCalculators

* Black format

* improve BaseDataTest

* add list format test

* make mapping_type() consistent

* fix else in __get_dict_data and __get_file_data

* Add docstrings to BaseData

* add DataCollection to_list test

* Update plusminus test

* Update BaseFormat convert example

* Update BaseFormat convert example

* Temporary RandomImageCalculator.py for CI fixing

* implement Optional hint

* update BaseFormat placeholder functions and correct typos

* Fix typos and tests for duplicating file output

* Update format return style

* Remove unnecessary tests

More comprehensive tests are added in PR #47

* Format correction

* Update ci.yml to include testing pytest scripts

* update docstrings of basecalculator

* clean up BaseCalculator

* finish BaseCalcualtor, working on Test

* fix base_dir

* cleaned up BaseCalculatorTest

* set DataCollection setitem

* fixed all tests

* add pytest for integration

* uncomment missed calculator test

* remove SpecializedCalculator.py

* add edit parameter tests in BaseCalculatorTest.py

* fix ci

* add | to ci.yml

* Parameters.to_dict(): Deepcopy to not modify the original parameters

* Update README

Add installation and testing instructions.

* Update BaseCalculatorTest

* Renamed 'pointer' to 'reference' (there are no pointers in python)
* Removed commented code

* Update Test.

Remove unused modules
Remove travis check
pep8 fixes

* Syntax fix

* Update BaseCalculatorTest

Removed logging since not used.

* Update BaseCalculatorTest

Adding class docstring to class.

* Update BaseCalculator

Typo in authors list
Remove implementation of init_parameters(), it's an abstract class, so raise
NotImplementedError.
TODO: test init_parameters()

* Update README

Make summary more concise.

* Update BaseCalculator

black reformatting.

* Update example notebook

Adjust to new API (libpyvinyl)
TODO: fix BaseCalculator, see error in cell 7)

* Add Parameter class to module.

* Seperate unit tests and integration tests

* update tests in README.md

* update ci.yml with the new test file structure

* Fix BaseCalculator output consistensy check

* check expected_data can be reached, then allow extra data to be read

* expose BaseData in libpyvinyl __init__

* update setup.py toward 0.1.0

* bump version in setup.py

* Update CI badge in README.md

Replace the outdated Travis badge with current github CI badge.

* Add Parameters test corresponding to #51

* Method to retrieve the parameter value as a pint Quantity

* Support for numpy floats as input with conversion to pint.Quantity

* numpy now required by Parameter class in order to accept numpy floats as value

* Allow BaseCalculator to accept None input.

* Fix #50 by increasing ljust and adding a space

To deal with very long parameter names, we increasted the ljust and
added a space after the name, value and unit.

* Remove example calculators, which are not compatible with the new BaseCalculator class

* Remove unnecessary docs of the integration test

- Remove the license, history.rst and contributing
- Add graphs explaining the plusminus integration test

* Fix setup.py author format

Co-authored-by: Carsten Fortmann-Grote <[email protected]>
Co-authored-by: Carsten Fortmann-Grote <[email protected]>
Co-authored-by: Shervin Nourbakhsh <[email protected]>
Co-authored-by: Carsten Fortmann-Grote <[email protected]>
Co-authored-by: Mads Bertelsen <[email protected]>
Co-authored-by: mads-bertelsen <[email protected]>
Co-authored-by: shervin86 <[email protected]>
shervin86 pushed a commit that referenced this issue Apr 7, 2022
@JunCEEE
Add Parameters test corresponding to #51
985fcf8
JunCEEE added a commit to JunCEEE/libpyvinyl that referenced this issue Apr 28, 2022
@JunCEEE
Add Parameters test corresponding to PaNOSC-ViNYL#51
858de38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@JunCEEE JunCEEE

@shervin86 shervin86

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@JunCEEE @shervin86