University-of-Reading-Space-Science / HUXt

HUXt - a lightweight solar wind model.
MIT License
37 stars 17 forks source link

conda and pip installation files #2

Closed cmoestl closed 4 years ago

cmoestl commented 4 years ago

Hi, very nice work! it would be best if you could add the environment.yml and requirements.txt files, e.g. see "Installation" in https://github.com/cmoestl/heliocats (with the files in this repository defining our "standard" enviroment which we call "helio") so one can immediately make an environment to run the code without the need to figure out which packages or different versions are needed.

edit: so now I run the code and after installing my "helio" environment + pip install h5py + pip install moviepy importing is fine; it then stops at model.solve(cme_list, save=True, tag='1d_conecme_test') with a numpy error as probably we use different numpy versions,

ciao chris

LukeBarnard commented 4 years ago

Hi Chris,

Thanks for this. You're right this would be better. It's been on the to-do list, but never quite made it to the top. I have some time to look at it this week though.

Can you let me know what version of numpy you are using and what the error message was?

Thanks for your help,

Cheers,

cmoestl commented 4 years ago

I tried it with numpy 1.18.1, and also numpy 1.16.2 (in my helio environment) but I always get this error, see screenshot Bildschirmfoto 2020-03-23 um 09 07 50

This is a basic environment.yml file that works for importing (no pip required):

name: huxt dependencies:

LukeBarnard commented 4 years ago

Thanks Chris - I'll have a look at this today.

STBadman commented 4 years ago

Agreed, very nice work! I was initially getting the same error as above running through the demo notebook. The issue was some instances of np.interp in which the value to be interpolated was an np.array containing astropy.unit.Quantity objects, e.g. line 854 :

v_boundary_update = np.interp(self.lon.value, lon_tstep.value, v_boundary_cone)

np.interp seemed to strip the units away, so v_boundary_cone entered with units km/s but v_boundary_update became just a normal np.array of floats, and then your save method runs into problems because it expects all the arrays it encounters to have units.

I updated numpy and astropy (now versions numpy.version : 1.18.1, astropy.version = 4.0) and now it seems to work as packaged. I agree an environment file would be handy for consistency.

LukeBarnard commented 4 years ago

Hey,

Thanks both for your feedback here, it's apprecated. I've added the requirements.txt and environment.yml files and updated the README. Hopefully this makes things a bit clearer now. I'll admit I don't tend to use virtual environments, so I was a bit slow to catch on.

On my machine, creating the conda environment and running the notebook examples works OK. Please let me know if you have any further problems!

Cheers,

cmoestl commented 4 years ago

the example notebook works like a charm on MacOSX with this environment, only needed to change the paths to / in config.dat, thanks much!

LukeBarnard commented 4 years ago

Great. Thanks for checking. I'll leave this open for a while to see if any similar problems occur.